fix mozilla#961: add csrf tokens to all preferences forms

smileys2 · May 23, 2019 · f3d81c4 · f3d81c4
1 parent 403a1c3
commit f3d81c4
Show file tree

Hide file tree

Showing 8 changed files with 20 additions and 14 deletions.
diff --git a/controllers/user.js b/controllers/user.js
@@ -268,6 +268,7 @@ async function getRemoveFxm(req, res) {
     title: req.fluentFormat("remove-fxm"),
     subscriber: sessionUser,
     whichPartial: "subpages/remove_fxm",
+    csrfToken: req.csrfToken(),
   });
 }
 
@@ -313,6 +314,7 @@ async function getPreferences(req, res) {
   res.render("dashboards", {
     title: "Firefox Monitor",
     whichPartial: "dashboards/preferences",
+    csrfToken: req.csrfToken(),
     verifiedEmails, unverifiedEmails,
   });
 }

diff --git a/routes/user.js b/routes/user.js
@@ -2,6 +2,7 @@
 
 const express = require("express");
 const bodyParser = require("body-parser");
+const csrf = require("csurf");
 
 const { asyncMiddleware } = require("../middleware");
 const {
@@ -13,20 +14,21 @@ const {
 const router = express.Router();
 const jsonParser = bodyParser.json();
 const urlEncodedParser = bodyParser.urlencoded({ extended: false });
+const csrfProtection = csrf();
 
 
 router.get("/dashboard", asyncMiddleware(getDashboard));
-router.get("/preferences", asyncMiddleware(getPreferences));
+router.get("/preferences", csrfProtection, asyncMiddleware(getPreferences));
 router.get("/logout", logout);
-router.post("/email", urlEncodedParser, asyncMiddleware(add));
-router.post("/remove-email", urlEncodedParser, asyncMiddleware(removeEmail));
-router.post("/resend-email", jsonParser, asyncMiddleware(resendEmail));
-router.post("/update-comm-option", jsonParser, asyncMiddleware(updateCommunicationOptions));
+router.post("/email", urlEncodedParser, csrfProtection, asyncMiddleware(add));
+router.post("/remove-email", urlEncodedParser, csrfProtection, asyncMiddleware(removeEmail));
+router.post("/resend-email", jsonParser, csrfProtection, asyncMiddleware(resendEmail));
+router.post("/update-comm-option", jsonParser, csrfProtection, asyncMiddleware(updateCommunicationOptions));
 router.get("/verify", jsonParser, asyncMiddleware(verify));
 router.use("/unsubscribe", urlEncodedParser);
 router.get("/unsubscribe", asyncMiddleware(getUnsubscribe));
-router.post("/unsubscribe", asyncMiddleware(postUnsubscribe));
-router.get("/remove-fxm", urlEncodedParser, asyncMiddleware(getRemoveFxm));
-router.post("/remove-fxm", jsonParser, asyncMiddleware(postRemoveFxm));
+router.post("/unsubscribe", csrfProtection, asyncMiddleware(postUnsubscribe));
+router.get("/remove-fxm", urlEncodedParser, csrfProtection, asyncMiddleware(getRemoveFxm));
+router.post("/remove-fxm", jsonParser, csrfProtection, asyncMiddleware(postRemoveFxm));
 
 module.exports = router;
diff --git a/tests/controllers/user.test.js b/tests/controllers/user.test.js
@@ -351,7 +351,7 @@ test("user/remove-fxm GET request with invalid session returns error", async ()
 
 test("user/remove-fxm GET request with valid session returns 200 and renders remove_fxm", async () => {
   // Set up mocks
-  const req = { fluentFormat: jest.fn(), session: { user: TEST_SUBSCRIBERS.firefox_account }};
+  const req = { fluentFormat: jest.fn(), csrfToken: jest.fn(), session: { user: TEST_SUBSCRIBERS.firefox_account }};
   const resp = httpMocks.createResponse();
   resp.render = jest.fn();
 

diff --git a/views/partials/dashboards/preferences.hbs b/views/partials/dashboards/preferences.hbs
@@ -13,7 +13,7 @@
         <section>
           {{#each communicationOptions}}
             <label class="radio-container">
-              <input class="radio-comm-option" data-comm-option="{{ optionId }}" data-form-action="update-comm-option" type="radio" {{ optionChecked }} name="1">
+              <input class="radio-comm-option" data-comm-option="{{ optionId }}" data-form-action="update-comm-option" data-csrf-token="{{ ../csrfToken }}" type="radio" {{ optionChecked }} name="1">
               <span class="radio-label">{{{ labelString }}}</span>
               <span class="checkmark"></span>
             </label>
@@ -33,7 +33,7 @@
       </div>
       <div class="pref">
         <h4 class="email-pref add-new">{{ getString "add-new-email" }}</h4>
-        {{> forms/add-another-email-form }}
+        {{> forms/add-another-email-form csrfToken=../csrfToken}}
       </div>
       <div class="pref remove">
         <a href="/user/remove-fxm"><h3 class="remove-fxm subhead">{{ getString "remove-fxm" }}{{> svg/arrow-head-right }}</h3></a>

diff --git a/views/partials/email-card.hbs b/views/partials/email-card.hbs
@@ -24,7 +24,7 @@
                 {{else}}
                   <!-- show resend-email link for unverified emails -->
                   <div class="button-resend-wrapper flx">
-                    <button class="resend-email" data-email-id="{{ id }}" data-form-action="resend-email">{{ getString "resend-verification" }}</button>
+                    <button class="resend-email" data-email-id="{{ id }}" data-form-action="resend-email" data-csrf-token="{{ csrfToken }}">{{ getString "resend-verification" }}</button>
                     <span class="sending hide"> {{getString "signup-modal-sent"}}</span>
                   </div>
                 {{/if}}
@@ -34,7 +34,7 @@
                 {{#if primary}}
                   <a href="{{ getFxaUrl }}" class="change-primary-email flx hide-mobile">{{ getString "link-change-primary" }}</a>
                 {{else}}
-                  {{> forms/remove-email-form }}
+                  {{> forms/remove-email-form csrfToken=../../../csrfToken }}
                 {{/if}}
               {{else}} <!-- user breaches dashboard / show toggle -->
                 {{#ifCompare breaches.length ">" 0}}

diff --git a/views/partials/forms/add-another-email-form.hbs b/views/partials/forms/add-another-email-form.hbs
@@ -1,4 +1,5 @@
             <form action="/user/email" method="post" id="add-another-email-form" class="form-group add-new-email-form" data-event-category="Add Another Email Form">
+              <input type="hidden" name="_csrf" value="{{ csrfToken }}">
               <div class="input-group">
                 <input id="email-add" class="input-group-field email-add" type="email" name="email" placeholder="{{ getString "scan-placeholder"}}" aria-label="{{ getString "scan-placeholder"}}" autocomplete="off">
                 {{> forms/error-message }}

diff --git a/views/partials/forms/remove-email-form.hbs b/views/partials/forms/remove-email-form.hbs
@@ -1,4 +1,5 @@
                   <form action="/user/remove-email" method="POST" class="remove-email svg-wrap" aria-label="{{ getString "stop-monitor-this" }}">
                     <input type="hidden" name="emailId" value="{{ id }}" />
+                    <input type="hidden" name="_csrf" value="{{ csrfToken }}">
                     <input class="remove-email-submit" type="submit" value="" />{{> svg/trash-can}}
                   </form>
diff --git a/views/partials/subpages/remove_fxm.hbs b/views/partials/subpages/remove_fxm.hbs
@@ -1,5 +1,5 @@
 <section id="unsubscribe" class="half">
     <h3 class="pref-section-headline remove">{{ getString "remove-fxm" }}</h3>
     <p class="subhead">{{ getString "remove-fxm-blurb" }}</p>
-    <button class="remove-fxm flx" data-form-action="remove-fxm" data-primary-token="{{ primaryToken }}" data-primary-hash="{{ primaryHash }}" href="#">{{ getString "remove-fxm" }}{{> svg/arrow-head-right }}</button>
+    <button class="remove-fxm flx" data-form-action="remove-fxm" data-csrf-token="{{ csrfToken }}" data-primary-token="{{ primaryToken }}" data-primary-hash="{{ primaryHash }}" href="#">{{ getString "remove-fxm" }}{{> svg/arrow-head-right }}</button>
 </section>