Review updates / Resolve conflicts

smileys2 · Feb 14, 2019 · 20afd2f · 20afd2f
1 parent df5871c
commit 20afd2f
Show file tree

Hide file tree

Showing 10 changed files with 165 additions and 51 deletions.
diff --git a/controllers/home.js b/controllers/home.js
@@ -1,9 +1,17 @@
 "use strict";
 
+const AppConstants = require("../app-constants");
 const scanResult = require("../scan-results");
+const { generatePageToken } = require("./utils");
+
 
 async function home(req, res) {
 
+  const formTokens = {
+    pageToken: AppConstants.PAGE_TOKEN_TIMER > 0 ? generatePageToken(req) : "",
+    csrfToken: req.csrfToken(),
+  };
+
   let featuredBreach = null;
   let scanFeaturedBreach = false;
 
@@ -20,8 +28,9 @@ async function home(req, res) {
     }
 
     const scanRes = await scanResult(req);
-    if (scanRes.doorhangerScan === true) {
-      return res.render("scan", scanRes);
+
+    if (scanRes.doorhangerScan) {
+      return res.render("scan", Object.assign(scanRes, formTokens));
     }
     scanFeaturedBreach = true;
   }
@@ -30,6 +39,8 @@ async function home(req, res) {
     title: req.fluentFormat("home-title"),
     featuredBreach: featuredBreach,
     scanFeaturedBreach,
+    pageToken: formTokens.pageToken,
+    csrfToken: formTokens.csrfToken,
   });
 }
 

diff --git a/controllers/scan.js b/controllers/scan.js
@@ -1,38 +1,106 @@
 "use strict";
 
-const sha1 = require("../sha1-utils");
+const crypto = require("crypto");
+
+const AppConstants = require("../app-constants");
+const { FluentError } = require("../locale-utils");
+const { generatePageToken } = require("./utils");
+const mozlog = require("../log");
 const scanResult = require("../scan-results");
+const sha1 = require("../sha1-utils");
+
+const log = mozlog("controllers.scan");
+
+function _decryptPageToken(encryptedPageToken) {
+  const decipher = crypto.createDecipher("aes-256-cbc", AppConstants.COOKIE_SECRET);
+  const decryptedPageToken = JSON.parse([decipher.update(encryptedPageToken, "base64", "utf8"), decipher.final("utf8")].join(""));
+  return decryptedPageToken;
+}
+
+
+function _validatePageToken(pageToken, req) {
+  const requestIP = req.headers["x-real-ip"] || req.ip;
+  const pageTokenIP = pageToken.ip;
+  if (pageToken.ip !== requestIP) {
+    log.error("_validatePageToken", {msg: "IP mis-match", pageTokenIP, requestIP});
+    return false;
+  }
+  if (Date.now() - new Date(pageToken.date) >= AppConstants.PAGE_TOKEN_TIMER * 1000) {
+    log.error("_validatePageToken", {msg: "expired pageToken"});
+    return false;
+  }
+  /* TODO: block on scans-per-ip instead of scans-per-timespan
+  if (req.session.scans.length > 5) {
+    console.log("too many scans this session");
+    return res.render("error");
+  }
+  if (!req.session.scans.includes(emailHash)) {
+    console.log(`adding ${emailHash} to session scans`);
+    req.session.scans.push(emailHash);
+  }
+  */
+  return pageToken;
+}
 
 
 async function post (req, res) {
   const emailHash = req.body.emailHash;
+  const encryptedPageToken = req.body.pageToken;
+  let validPageToken = false;
+
+  // for #688: use a page token to check that scans come from real pages
+  if (AppConstants.PAGE_TOKEN_TIMER > 0) {
+    if (!encryptedPageToken) {
+      throw new FluentError("error-scan-page-token");
+    }
+    const decryptedPageToken = _decryptPageToken(encryptedPageToken);
+    validPageToken = _validatePageToken(decryptedPageToken, req);
+
+    if (!validPageToken) {
+      throw new FluentError("error-scan-page-token");
+    }
+  }
 
   if (!emailHash || emailHash === sha1("")) {
     return res.redirect("/");
   }
 
   const scanRes = await scanResult(req);
+
+  const formTokens = {
+    pageToken: encryptedPageToken,
+    csrfToken: req.csrfToken(),
+  };
+
   if (req.session.user && scanRes.selfScan && !req.body.featuredBreach) {
     return res.redirect("/scan/user_dashboard");
   }
-  res.render("scan", scanRes);
+  res.render("scan", Object.assign(scanRes, formTokens));
 }
 
 
 async function getFullReport(req, res) {
   if (!req.session.user) {
     return res.redirect("/");
   }
+
   const scanRes = await scanResult(req, true);
   res.render("scan", scanRes);
 }
 
 
+
 async function getUserDashboard(req, res) {
+
   if (!req.session.user) {
     return res.redirect("/");
   }
 
+  const formTokens = {
+    pageToken: AppConstants.PAGE_TOKEN_TIMER > 0 ? generatePageToken(req) : "",
+    csrfToken: req.csrfToken(),
+  };
+
   const scanRes = await scanResult(req, true);
   scanRes.newUser = false;
 
@@ -41,7 +109,7 @@ async function getUserDashboard(req, res) {
     req.session.newUser = false;
   }
 
-  res.render("scan", scanRes);
+  return res.render("scan", Object.assign(scanRes, formTokens));
 }
 
 

diff --git a/controllers/utils.js b/controllers/utils.js
@@ -0,0 +1,26 @@
+"use strict";
+
+const crypto = require("crypto");
+const uuidv4 = require("uuid/v4");
+
+const AppConstants = require("../app-constants");
+
+
+function generatePageToken(req) {
+  const pageToken = {ip: req.ip, date: new Date(), nonce: uuidv4()};
+  const cipher = crypto.createCipher("aes-256-cbc", AppConstants.COOKIE_SECRET);
+  const encryptedPageToken = [cipher.update(JSON.stringify(pageToken), "utf8", "base64"), cipher.final("base64")].join("");
+  return encryptedPageToken;
+
+  /* TODO: block on scans-per-ip instead of scans-per-timespan
+  if (req.session.scans === undefined){
+    console.log("session scans undefined");
+    req.session.scans = [];
+  }
+  req.session.numScans = req.session.scans.length;
+  */
+}
+
+module.exports = {
+  generatePageToken,
+};
diff --git a/public/css/fxa.css b/public/css/fxa.css
@@ -1036,13 +1036,13 @@ main.scan-results .third {
 
 main.scan-results .single-breach {
   display: block;
-  margin-top: 0px;
+  margin-top: 0;
 }
 
 main.scan-results .single-breach .half {
   margin-left: 0;
   margin-right: 0;
-  margin-top: 0px;
+  margin-top: 0;
 }
 
 .button-link-wrapper {

diff --git a/routes/scan.js b/routes/scan.js
@@ -2,17 +2,20 @@
 
 const express = require("express");
 const bodyParser = require("body-parser");
+const csrf = require("csurf");
 
 const {asyncMiddleware} = require("../middleware");
 const {post, get, getFullReport, getUserDashboard} = require("../controllers/scan");
 
 
 const router = express.Router();
 const urlEncodedParser = bodyParser.urlencoded({ extended: false });
+const csrfProtection = csrf();
 
-router.post("/", urlEncodedParser, asyncMiddleware(post));
+
+router.post("/", urlEncodedParser, csrfProtection, asyncMiddleware(post));
 router.get("/", get);
 router.get("/full_report", getFullReport);
-router.get("/user_dashboard", getUserDashboard);
+router.get("/user_dashboard", urlEncodedParser, csrfProtection, asyncMiddleware(getUserDashboard));
 
 module.exports = router;
diff --git a/scan-results.js b/scan-results.js
@@ -5,11 +5,13 @@ const { URL } = require("url");
 const HIBP = require("./hibp");
 const sha1 = require("./sha1-utils");
 
+
 const scanResult = async(req, selfScan=false) => {
 
   const allBreaches = req.app.locals.breaches;
   let scannedEmail = null;
 
+  const title = req.fluentFormat("scan-title");
   let foundBreaches = [];
   let specificBreach = null;
   let doorhangerScan = false;
@@ -26,33 +28,28 @@ const scanResult = async(req, selfScan=false) => {
   // Checks if the user scanning their own verified email.
   if (req.body && req.body.emailHash) {
     scannedEmail = req.body.emailHash;
-    if (!selfScan && signedInUser) {
-      if (sha1(signedInUser.email).toUpperCase() === req.body.emailHash) {
-        selfScan = true;
-      }
+    if (!selfScan && signedInUser && sha1(signedInUser.email) === req.body.emailHash) {
+      selfScan = true;
     }
   }
 
   const url = new URL(req.url, req.app.locals.SERVER_URL);
+  const thisBreach = (breach) => {
+    return (element) => element.Name.toLowerCase() === breach.toLowerCase();
+  };
 
   // Checks for a signedInUser arriving from doorhanger.
-  if (signedInUser) {
-    if (url.searchParams.has("utm_source") && url.searchParams.get("utm_source") === "firefox") {
-      doorhangerScan = true, selfScan = true;
-      specificBreach = allBreaches.find(breach => breach.Name.toLowerCase() === req.query.breach.toLowerCase());
-    }
+  if (signedInUser && url.searchParams.has("utm_source") && url.searchParams.get("utm_source") === "firefox") {
+    doorhangerScan = true, selfScan = true;
+    specificBreach = allBreaches.find(thisBreach(req.query.breach));
   }
 
-  if (url.pathname === "/full_report") {
-    fullReport = true;
-  }
+  fullReport = url.pathname === "/full_report";
 
-  if (url.pathname === "/user_dashboard") {
-    userDash = true;
-  }
+  userDash = url.pathname === "/user_dashboard";
 
   if (selfScan) {
-    scannedEmail = sha1(signedInUser.email).toUpperCase();
+    scannedEmail = sha1(signedInUser.email);
   }
 
   if (scannedEmail) {
@@ -62,7 +59,7 @@ const scanResult = async(req, selfScan=false) => {
 
   // Checks if scan originated from a breach detail/"featured breach" page.
   if (req.body && req.body.featuredBreach) {
-    specificBreach = allBreaches.find(breach => breach.Name.toLowerCase() === req.body.featuredBreach.toLowerCase());
+    specificBreach = allBreaches.find(thisBreach(req.body.featuredBreach));
   }
 
   if (doorhangerScan || specificBreach) {
@@ -72,23 +69,21 @@ const scanResult = async(req, selfScan=false) => {
     // brings specificBreach to front of foundBreaches list.
     if (specificBreachIndex !== -1) {
       userCompromised = true;
-      if (foundBreaches.length > 1) {
-        foundBreaches.splice(specificBreachIndex, 1);
-        foundBreaches.unshift(specificBreach);
-      }
+      foundBreaches.splice(specificBreachIndex, 1);
+      foundBreaches.unshift(specificBreach);
     }
   }
 
   return {
-    foundBreaches: foundBreaches,
-    specificBreach: specificBreach,
-    doorhangerScan: doorhangerScan,
-    userCompromised: userCompromised,
-    signedInUser: signedInUser,
-    selfScan: selfScan,
-    fullReport: fullReport,
-    userDash: userDash,
-    title: req.fluentFormat("scan-title"),
+    title,
+    foundBreaches,
+    specificBreach,
+    doorhangerScan,
+    userCompromised,
+    signedInUser,
+    selfScan,
+    fullReport,
+    userDash,
   };
 };
 

diff --git a/sha1-utils.js b/sha1-utils.js
@@ -3,7 +3,7 @@
 const crypto = require("crypto");
 
 function getSha1(email) {
-  return crypto.createHash("sha1").update(email).digest("hex");
+  return crypto.createHash("sha1").update(email).digest("hex").toUpperCase();
 }
 
 module.exports = getSha1;
diff --git a/tests/controllers/scan.test.js b/tests/controllers/scan.test.js
@@ -39,7 +39,7 @@ test("scan POST with hash should render scan with foundBreaches", async () => {
   mockRequest.body = { emailHash: sha1(testEmail) };
   mockRequest.app = { locals: { breaches: testBreaches } };
   mockRequest.session = { user: null };
-  mockRequest.url = { url: "https://www.mozilla.com" };
+  mockRequest.url = { url: AppConstants.SERVER_URL };
   mockRequest.app.locals.SERVER_URL = AppConstants.SERVER_URL;
   const mockResponse = { render: jest.fn() };
   HIBP.getBreachesForEmail.mockResolvedValue(testFoundBreaches);

diff --git a/views/partials/fxa_enabled/fxa_scan.hbs b/views/partials/fxa_enabled/fxa_scan.hbs
@@ -28,18 +28,29 @@
   {{#if signedInUser}}
     {{> banners/download_firefox_banner}}
   {{else}}
-    {{> banners/sign_up_banner class="extra-margin"}}
+    {{> banners/sign_up_banner}}
   {{/if}}
 {{else}}
+
   {{#if fullReport}}
     {{> what_to_do class="extra-margin"}}
   {{else}}
-    {{> what_to_do}}
-  {{/if}}
-  {{> scan_another_email}}
-  {{#if signedInUser}}
-    {{> banners/download_firefox_banner}}
-  {{else}}
-    {{> banners/sign_up_banner}}
+    {{#ifCompare foundBreaches.length "===" 1}}
+      {{> banners/sign_up_banner class="extra-margin"}}
+      {{> what_to_do}}
+      {{> scan_another_email}}
+      {{> banners/download_firefox_banner}}
+    {{/ifCompare}}
+
+    {{#ifCompare foundBreaches.length ">" 1}}
+      {{> what_to_do}}
+      {{> scan_another_email}}
+        {{#if signedInUser}}
+          {{> banners/download_firefox_banner}}
+        {{else}}
+          {{> banners/sign_up_banner}}
+        {{/if}}
+    {{/ifCompare}}
   {{/if}}
+
 {{/ifCompare}}
diff --git a/views/partials/fxa_enabled/scan_results/result_headline.hbs b/views/partials/fxa_enabled/scan_results/result_headline.hbs
@@ -1,9 +1,9 @@
   {{#ifCompare foundBreaches.length "===" 0}}
-    {{#ifCompare userDash "||" fullReport}}
+    {{#if selfScan}}
       <h3 id="latest-breaches" class="section-headline">{{fluentFormat req.supportedLocales "user-zero-breaches-headline" userEmail=signedInUser.email}}</h3>
     {{else}}
       <h3 id="no-breaches" class="section-headline">{{fluentFormat req.supportedLocales "guest-zero-breaches-headline"}}</h3>
-    {{/ifCompare}}
+    {{/if}}
   {{else}}
     <h3 id="scan-results-headline" class="section-headline scan-results-headline">
       {{#if userCompromised}}