fix mozilla#286: /user/unsubscribe route and logic

smileys2 · Aug 26, 2018 · 4455f95 · 4455f95
1 parent 8313d57
commit 4455f95
Show file tree

Hide file tree

Showing 11 changed files with 90 additions and 19 deletions.
diff --git a/controllers/ses.js b/controllers/ses.js
@@ -13,6 +13,7 @@ async function notification(req, res) {
       {status: "OK"}
     );
   } catch (e) {
+    console.error("SES notification error: ", e);
     res.status(500).json(
       {info: "Internal error."}
     );
@@ -52,7 +53,7 @@ async function handleComplaintMessage(message) {
 
 async function removeSubscribersFromDB(recipients) {
   for (const recipient of recipients) {
-    await DB.removeSubscriber(recipient.emailAddress);
+    await DB.removeSubscriberByEmail(recipient.emailAddress);
   }
 }
 

diff --git a/controllers/user.js b/controllers/user.js
@@ -2,7 +2,6 @@
 
 const isemail = require("isemail");
 
-const AppConstants = require("../app-constants");
 const DB = require("../db/DB");
 const EmailUtils = require("../email-utils");
 
@@ -12,16 +11,16 @@ async function add(req, res) {
   if (!isemail.validate(email)) {
     throw new Error("Invalid Email");
   }
-  const unverifiedSubscriber = await DB.addSubscriberUnverifiedEmailHash(email);
-  const token = unverifiedSubscriber.verification_token;
 
-  const url = `${AppConstants.SERVER_URL}/user/verify?token=${encodeURIComponent(token)}`;
+  const unverifiedSubscriber = await DB.addSubscriberUnverifiedEmailHash(email);
+  const verifyUrl = EmailUtils.verifyUrl(unverifiedSubscriber);
+  const unsubscribeUrl = EmailUtils.unsubscribeUrl(unverifiedSubscriber);
 
   await EmailUtils.sendEmail(
     email,
     "Verify your email address to subscribe to Firefox Monitor.",
     "email_verify",
-    { email, url}
+    { email, verifyUrl, unsubscribeUrl}
   );
 
   res.send({
@@ -40,10 +39,28 @@ async function verify(req, res) {
 }
 
 
-// TODO: create unsubscribe controller with token authentication
+function getUnsubscribe(req, res) {
+  res.render("unsubscribe", {
+    title: "Firefox Monitor: Unsubscribe",
+    token: req.query.token,
+    hash: req.query.hash,
+  });
+}
+
+
+async function postUnsubscribe(req, res) {
+  const unsubscribedUser = await DB.removeSubscriberByToken(req.body.token, req.body.emailHash);
+
+  res.render("unsubscribe", {
+    title: "Firefox Monitor: Unsubscribe",
+    unsubscribed: unsubscribedUser,
+  });
+}
 
 
 module.exports = {
   add,
   verify,
+  getUnsubscribe,
+  postUnsubscribe,
 };
diff --git a/db/DB.js b/db/DB.js
@@ -20,6 +20,16 @@ const DB = {
     return res[0];
   },
 
+  async getSubscriberByTokenAndHash(token, emailSha1) {
+    const res = await knex.table("subscribers")
+      .first()
+      .where({
+        "verification_token": token,
+        "sha1": emailSha1,
+      });
+    return res;
+  },
+
   async getSubscribersByEmail(email) {
     return await knex("subscribers")
       .where("email", "=", email);
@@ -84,17 +94,15 @@ const DB = {
 
   async _verifySubscriber(emailHash) {
     await HIBP.subscribeHash(emailHash.sha1);
-    // TODO: resolve with error if HIBP fails
     const verifiedSubscriber = await knex("subscribers")
       .where("email", "=", emailHash.email)
       .update({ verified: true })
       .returning("*");
     return verifiedSubscriber;
   },
 
-  async removeSubscriber(email) {
+  async removeSubscriberByEmail(email) {
     const sha1 = getSha1(email);
-
     return await this._getSha1EntryAndDo(sha1, async aEntry => {
       await knex("subscribers")
         .where("id", "=", aEntry.id)
@@ -107,6 +115,17 @@ const DB = {
     });
   },
 
+  async removeSubscriberByToken(token, emailSha1) {
+    const subscriber = await this.getSubscriberByTokenAndHash(token, emailSha1);
+    await knex("subscribers")
+      .where({
+        "verification_token": subscriber.verification_token,
+        "sha1": subscriber.sha1,
+      })
+      .del();
+    return subscriber;
+  },
+
   async getSubscribersByHashes(hashes) {
     return await knex("subscribers").whereIn("sha1", hashes).andWhere("verified", "=", true);
   },

diff --git a/email-utils.js b/email-utils.js
@@ -28,11 +28,7 @@ const EmailUtils = {
     // Allow a debug mode that will log JSON instead of sending emails.
     if (!smtpUrl) {
       console.info("smtpUrl is empty, EmailUtils will log a JSON response instead of sending emails.");
-      gTransporter = {
-        sendMail(options, callback) {
-          callback(null, "dummy mode");
-        },
-      };
+      gTransporter = nodemailer.createTransport({jsonTransport: true});
       return Promise.resolve(true);
     }
 
@@ -65,10 +61,22 @@ const EmailUtils = {
           reject(error);
           return;
         }
+        if (gTransporter.transporter.name === "JSONTransport") {
+          console.log(info.message.toString());
+        }
         resolve(info);
       });
     });
   },
+
+  verifyUrl (subscriber) {
+    return `${AppConstants.SERVER_URL}/user/verify?token=${encodeURIComponent(subscriber.verification_token)}`;
+  },
+
+  unsubscribeUrl (subscriber) {
+    return `${AppConstants.SERVER_URL}/user/unsubscribe?token=${encodeURIComponent(subscriber.verification_token)}&hash=${encodeURIComponent(subscriber.sha1)}`;
+  },
+
 };
 
 module.exports = EmailUtils;
diff --git a/public/js/monitor.js b/public/js/monitor.js
@@ -193,6 +193,10 @@ const postData = (url, data = {}) => {
 };
 
 function handleFormSubmits(formEvent) {
+  if (formEvent.target.id === "unsubscribe-form") {
+    // default form submit behavior for unsubscribe-form
+    return;
+  }
   formEvent.preventDefault();
   const thisForm = formEvent.target;
   if (!thisForm.email.value || !isValidEmail(thisForm.email.value)) {

diff --git a/routes/user.js b/routes/user.js
@@ -4,14 +4,18 @@ const express = require("express");
 const bodyParser = require("body-parser");
 
 const { asyncMiddleware } = require("../middleware");
-const { add, verify } = require("../controllers/user");
+const { add, verify, getUnsubscribe, postUnsubscribe } = require("../controllers/user");
 
 const router = express.Router();
 const jsonParser = bodyParser.json();
+const urlEncodedParser = bodyParser.urlencoded({ extended: false });
 
 
 router.post("/add", jsonParser, asyncMiddleware(add));
 router.get("/verify", jsonParser, asyncMiddleware(verify));
+router.use("/unsubscribe", urlEncodedParser);
+router.get("/unsubscribe", getUnsubscribe);
+router.post("/unsubscribe", asyncMiddleware(postUnsubscribe));
 
 
 module.exports = router;
diff --git a/tests/db.test.js b/tests/db.test.js
@@ -74,7 +74,7 @@ test("removeSubscriber accepts email and removes the email address", async () =>
   let subscribers = await DB.getSubscribersByHashes([getSha1(testEmail)]);
   expect(subscribers.length).toEqual(1);
 
-  await DB.removeSubscriber(verifiedSubscriber.email);
+  await DB.removeSubscriberByEmail(verifiedSubscriber.email);
   subscribers = await DB.getSubscribersByHashes([getSha1(testEmail)]);
   expect(subscribers.length).toEqual(0);
 });
diff --git a/tests/email.test.js b/tests/email.test.js
@@ -13,7 +13,9 @@ test("EmailUtils.init with empty host doesnt invoke nodemailer", () => {
 
   EmailUtils.init("");
 
-  expect(nodemailer.createTransport.mock.calls.length).toBe(0);
+  const mockCreateTransport = nodemailer.createTransport.mock;
+  expect(mockCreateTransport.calls.length).toBe(1);
+  expect(mockCreateTransport.calls[0][0]).toEqual({jsonTransport: true});
 });
 
 

diff --git a/views/email/email_verify.hbs b/views/email/email_verify.hbs
@@ -3,4 +3,4 @@
 
   <p>To verify your subscription, please visit this link:</p>
 
-  {{ url }}
+  {{ verifyUrl }}
diff --git a/views/layouts/email.hbs b/views/layouts/email.hbs
@@ -27,5 +27,6 @@
     </tr>
   </table>
 
+You got this email because you signed up at {{ AppConstants.SERVER_URL }}. <a href="{{ unsubscribeUrl }}" rel="noreferrer noopener">Unsubscribe</a>
 </body>
 </html>
diff --git a/views/unsubscribe.hbs b/views/unsubscribe.hbs
@@ -0,0 +1,15 @@
+{{!< default }}
+<div class="unsubscribe-message-wrapper">
+  <div class="unsubscribe-message">
+      {{#if unsubscribed }}
+          <h4>You have unsubscribed.</h4>
+      {{ else }}
+          <h4>Are you sure you want to unsubscribe?</h4>
+          <form action="/user/unsubscribe" method="post" id="unsubscribe-form">
+              <input type="hidden" name="token" value="{{ token }}">
+              <input type="hidden" name="emailHash" value="{{ hash }}">
+              <input type="submit" class="button" value="Unsubscribe">
+          </form>
+      {{/if}}
+  </div>
+</div>
Original file line number	Diff line number	Diff line change
Expand Up		@@ -3,4 +3,4 @@

		<p>To verify your subscription, please visit this link:</p>

		{{ url }}
		{{ verifyUrl }}
-Original file line number
+Diff line change
@@ Expand Up / @@ -27,5 +27,6 @@ @@
         </tr>
       </table>
+    You got this email because you signed up at {{ AppConstants.SERVER_URL }}. <a href="{{ unsubscribeUrl }}" rel="noreferrer noopener">Unsubscribe</a>
     </body>
     </html>