fix mozilla#941: implement remove button

smileys2 · May 22, 2019 · 267d134 · 267d134
1 parent e970f7b
commit 267d134
Show file tree

Hide file tree

Showing 10 changed files with 99 additions and 87 deletions.
diff --git a/controllers/user.js b/controllers/user.js
@@ -1,6 +1,5 @@
 "use strict";
 
-const crypto = require("crypto");
 const isemail = require("isemail");
 
 
@@ -226,45 +225,73 @@ async function verify(req, res) {
 }
 
 
+// legacy /user/unsubscribe controller for pre-FxA unsubscribe links
 async function getUnsubscribe(req, res) {
   if (!req.query.token) {
     throw new FluentError("user-unsubscribe-token-error");
   }
 
   const subscriber = await DB.getSubscriberByToken(req.query.token);
+  // Token is for a primary email address,
+  // redirect to preferences to remove Firefox Monitor
+  if (subscriber) {
+    return res.redirect("/user/preferences");
+  }
 
-  //throws error if user backs into and refreshes unsubscribe page
-  if (!subscriber) {
+  const emailAddress = await DB.getEmailByToken(req.query.token);
+  if (!subscriber && !emailAddress) {
     throw new FluentError("error-not-subscribed");
   }
 
   res.render("subpage", {
     title: req.fluentFormat("user-unsubscribe-title"),
-    headline: req.fluentFormat("unsub-headline"),
-    subhead: req.fluentFormat("unsub-blurb"),
     whichPartial: "subpages/unsubscribe",
     token: req.query.token,
     hash: req.query.hash,
   });
 }
 
 
+async function getRemoveFxm(req, res) {
+  const sessionUser = _requireSessionUser(req);
+
+  res.render("subpage", {
+    title: req.fluentFormat("remove-fxm"),
+    subscriber: sessionUser,
+    whichPartial: "subpages/remove_fxm",
+  });
+}
+
+
+async function postRemoveFxm(req, res) {
+  const sessionUser = _requireSessionUser(req);
+  await DB.removeSubscriber(sessionUser);
+
+  req.session.reset();
+  res.redirect("/");
+}
+
+
 async function postUnsubscribe(req, res) {
-  if (!req.body.token || !req.body.emailHash) {
+  const { token, emailHash } = req.body;
+
+  if (!token || !emailHash) {
     throw new FluentError("user-unsubscribe-token-email-error");
   }
-  const unsubscribedUser = await DB.removeSubscriberByToken(req.body.token, req.body.emailHash);
-  await FXA.revokeOAuthToken(unsubscribedUser.fxa_refresh_token);
 
-  // if user backs into unsubscribe page and clicks "unsubscribe" again
+  // legacy unsubscribe link page uses removeSubscriberByToken
+  const unsubscribedUser = await DB.removeSubscriberByToken(token, emailHash);
   if (!unsubscribedUser) {
-    throw new FluentError("error-not-subscribed");
+    const emailAddress = await DB.getEmailByToken(token);
+    if (!emailAddress) {
+      throw new FluentError("error-not-subscribed");
+    }
+    await DB.removeOneSecondaryEmail(emailAddress.id);
+    return res.redirect("/user/preferences");
   }
-
-  const surveyTicket = crypto.randomBytes(40).toString("hex");
-  req.session.unsub = surveyTicket;
-
-  res.redirect("unsubscribe_survey");
+  await FXA.revokeOAuthToken(unsubscribedUser.fxa_refresh_token);
+  req.session.reset();
+  res.redirect("/");
 }
 
 
@@ -282,28 +309,6 @@ async function getPreferences(req, res) {
 }
 
 
-function getUnsubSurvey(req, res) {
-  //throws error if user refreshes unsubscribe survey page after they have submitted an answer
-  if(!req.session.unsub) {
-    throw new FluentError("error-not-subscribed");
-  }
-  res.render("subpage", {
-    title: req.fluentFormat("user-unsubscribe-survey-title"),
-    headline: req.fluentFormat("unsub-survey-headline-v2"),
-    subhead: req.fluentFormat("unsub-survey-blurb-v2"),
-    whichPartial: "subpages/unsubscribe_survey",
-  });
-}
-
-
-function postUnsubSurvey(req, res) {
-  //clear session in case a user subscribes / unsubscribes multiple times or with multiple email addresses.
-  req.session.reset();
-  res.send({
-    title: req.fluentFormat("user-unsubscribed-title"),
-  });
-}
-
 function logout(req, res) {
   req.session.reset();
   res.redirect("/");
@@ -317,8 +322,8 @@ module.exports = {
   verify,
   getUnsubscribe,
   postUnsubscribe,
-  getUnsubSurvey,
-  postUnsubSurvey,
+  getRemoveFxm,
+  postRemoveFxm,
   logout,
   removeEmail,
   resendEmail,

diff --git a/db/DB.js b/db/DB.js
@@ -259,6 +259,11 @@ const DB = {
     return updatedSubscriber;
   },
 
+  async removeSubscriber(subscriber) {
+    await knex("email_addresses").where({"subscriber_id": subscriber.id}).del();
+    await knex("subscribers").where({"id": subscriber.id}).del();
+  },
+
   async removeSubscriberByEmail(email) {
     const sha1 = getSha1(email);
     return await this._getSha1EntryAndDo(sha1, async aEntry => {

diff --git a/public/js/dashboard.js b/public/js/dashboard.js
@@ -6,28 +6,29 @@ async function sendForm(action, formBody={}) {
     headers: {
       "Content-Type": "application/json; charset=utf-8",
     },
-    mode: "cors",
+    mode: "same-origin",
     method: "POST",
     body: JSON.stringify(formBody),
   });
 
+  if (response.redirected) {
+    window.location = response.url;
+    return;
+  }
   return await response.json();
 }
 
 async function sendCommunicationOption(e) {
-  const radioButton = e.target;
-  const formAction = radioButton.dataset.formAction;
-  const option = radioButton.dataset.commOption;
-  sendForm(formAction, { communicationOption: option })
+  const { formAction, commOption } = e.target.dataset;
+  sendForm(formAction, { communicationOption: commOption })
     .then(data => {}) /*decide what to do with data */
     .catch(e => {})/* decide how to handle errors */;
 }
 
 async function resendEmail(e) {
   const resendEmailBtn = e.target;
+  const { formAction, emailId } = resendEmailBtn.dataset;
   resendEmailBtn.classList.add("email-sent");
-  const emailId =  resendEmailBtn.dataset.emailId;
-  const formAction = resendEmailBtn.dataset.formAction;
 
   await sendForm(formAction, { emailId: emailId })
   .then(data => {
@@ -51,3 +52,11 @@ if (document.querySelector(".email-card")) {
     option.addEventListener("click", sendCommunicationOption);
   });
 }
+
+if (document.querySelector(".remove-fxm")) {
+  const removeMonitorButton = document.querySelector(".remove-fxm");
+  removeMonitorButton.addEventListener("click", async (e) => {
+    const {formAction, primaryToken, primaryHash} = e.target.dataset;
+    await sendForm(formAction, {primaryToken, primaryHash});
+  });
+}
diff --git a/routes/user.js b/routes/user.js
@@ -4,7 +4,11 @@ const express = require("express");
 const bodyParser = require("body-parser");
 
 const { asyncMiddleware } = require("../middleware");
-const { getDashboard,getPreferences, add, verify, getUnsubscribe, postUnsubscribe, getUnsubSurvey, postUnsubSurvey, removeEmail, resendEmail, updateCommunicationOptions, logout } = require("../controllers/user");
+const {
+  add, verify, logout,
+  getDashboard, getPreferences, removeEmail, resendEmail, updateCommunicationOptions,
+  getUnsubscribe, postUnsubscribe, getRemoveFxm, postRemoveFxm,
+} = require("../controllers/user");
 
 const router = express.Router();
 const jsonParser = bodyParser.json();
@@ -19,10 +23,10 @@ router.post("/remove-email", urlEncodedParser, asyncMiddleware(removeEmail));
 router.post("/resend-email", jsonParser, asyncMiddleware(resendEmail));
 router.post("/update-comm-option", jsonParser, asyncMiddleware(updateCommunicationOptions));
 router.get("/verify", jsonParser, asyncMiddleware(verify));
-router.use("/email/unsubscribe", urlEncodedParser);
-router.get("/email/unsubscribe", asyncMiddleware(getUnsubscribe));
-router.post("/email/unsubscribe", asyncMiddleware(postUnsubscribe));
-router.get("/email/unsubscribe_survey", getUnsubSurvey);
-router.post("/email/unsubscribe_survey", jsonParser, postUnsubSurvey);
+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));
 
 module.exports = router;
diff --git a/server.js b/server.js
@@ -156,8 +156,8 @@ app.locals.UTM_SOURCE = new URL(AppConstants.SERVER_URL).hostname;
 app.use(sessions({
   cookieName: "session",
   secret: AppConstants.COOKIE_SECRET,
-  duration: 15 * 60 * 1000, // 15 minutes
-  activeDuration: 5 * 60 * 1000, // 5 minutes
+  duration: 60 * 60 * 1000, // 60 minutes
+  activeDuration: 15 * 60 * 1000, // 15 minutes
   cookie: cookie,
 }));
 

diff --git a/tests/controllers/user.test.js b/tests/controllers/user.test.js
@@ -246,7 +246,7 @@ test("user verify request with invalid token returns error", async () => {
 });
 
 
-test("user unsubscribe GET request with valid token and hash returns 200 without error", async () => {
+test("user unsubscribe GET request with valid token and hash for primary/subscriber record returns 302 to preferences", async () => {
   // from db/seeds/test_subscribers.js
   const subscriberToken = TEST_SUBSCRIBERS.firefox_account.primary_verification_token;
   const subscriberHash = getSha1(TEST_SUBSCRIBERS.firefox_account.primary_email);
@@ -258,10 +258,16 @@ test("user unsubscribe GET request with valid token and hash returns 200 without
   // Call code-under-test
   await user.getUnsubscribe(req, resp);
 
-  expect(resp.statusCode).toEqual(200);
+  expect(resp.statusCode).toEqual(302);
+  expect(resp._getRedirectUrl()).toEqual("/user/preferences");
 });
 
 
+// TODO: test("remove-email POST");
+// TODO: test("remove-fxm get");
+// TODO: test("remove-fxm POST");
+
+
 test("user unsubscribe GET request with invalid token returns error", async () => {
   const invalidToken = "123456789";
 
@@ -276,23 +282,21 @@ test("user unsubscribe GET request with invalid token returns error", async () =
 });
 
 
-test("user unsubscribe POST request with valid hash and token unsubscribes user and calls FXA.revokeOAuthToken", async () => {
-  const validToken = TEST_SUBSCRIBERS.unverified_email.primary_verification_token;
-  const validHash = getSha1(TEST_SUBSCRIBERS.unverified_email.primary_email);
+test("user unsubscribe POST request with valid hash and token for email_address removes from DB", async () => {
+  const validToken = TEST_EMAIL_ADDRESSES.firefox_account.verification_token;
+  const validHash = TEST_EMAIL_ADDRESSES.firefox_account.sha1;
 
   // Set up mocks
   const req = { fluentFormat: jest.fn(), body: { token: validToken, emailHash: validHash }, session: {}};
   const resp = httpMocks.createResponse();
-  FXA.revokeOAuthToken = jest.fn();
 
   // Call code-under-test
   await user.postUnsubscribe(req, resp);
 
   expect(resp.statusCode).toEqual(302);
-  const subscriber = await DB.getSubscriberByToken(validToken);
-  expect(subscriber).toBeUndefined();
-  const mockCalls = FXA.revokeOAuthToken.mock.calls;
-  expect(mockCalls.length).toEqual(1);
+  expect(resp._getRedirectUrl()).toEqual("/user/preferences");
+  const emailAddress = await DB.getEmailByToken(validToken);
+  expect(emailAddress).toBeUndefined();
 });
 
 

diff --git a/views/partials/dashboards/preferences.hbs b/views/partials/dashboards/preferences.hbs
@@ -36,9 +36,7 @@
         {{> forms/add-another-email-form }}
       </div>
       <div class="pref remove">
-        <h3 class="pref-section-headline remove">{{ getString "remove-fxm" }}</h3>
-        <p class="subhead">{{ getString "remove-fxm-blurb" }}</p>
-        <button class="remove-fxm subhead flx" href="/">{{ getString "remove-fxm" }}{{> svg/arrow-head-right }}</button>
+        <a href="/user/remove-fxm"><h3 class="remove-fxm subhead">{{ getString "remove-fxm" }}{{> svg/arrow-head-right }}</h3></a>
       </div>
     </div>
   </section>

diff --git a/views/partials/subpages/remove_fxm.hbs b/views/partials/subpages/remove_fxm.hbs
@@ -0,0 +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 subhead flx" data-form-action="remove-fxm" data-primary-token="{{ primaryToken }}" data-primary-hash="{{ primaryHash }}" href="#">{{ getString "remove-fxm" }}{{> svg/arrow-head-right }}</button>
+</section>
diff --git a/views/partials/subpages/unsubscribe.hbs b/views/partials/subpages/unsubscribe.hbs
@@ -1,4 +1,6 @@
 <section id="unsubscribe" class="half">
+  <h3 class="pref-section-headline remove">{{{ getString "unsub-headline" }}}</h3>
+  <p class="subhead">{{{ getString "unsub-blurb" }}}</p>
   <form action="/user/unsubscribe" class="form-group" method="post" id="unsubscribe-form">
     <input type="hidden" name="token" value="{{ token }}">
     <input type="hidden" name="emailHash" value="{{ hash }}">

diff --git a/views/partials/subpages/unsubscribe_survey.hbs b/views/partials/subpages/unsubscribe_survey.hbs