fix(mailer): Several mailer fixes

* Password changed template shows "reset your password" link. "change your password" used to be displayed. If an attacker changed the user's password, the user would not know the new password to change it. * Fix the "reset password" link color in the "Your password has been reset" email. * All password reset links auto-submit the password reset form. * The "Change your password" link in the "New sign in to Firefox" email contains the users email address. fixes #151 fixes #153 fixes #154 fixes #155 fixes #157
2016-05-11 17:46:21 +01:00 · 2016-05-11 17:46:21 +01:00 · a9d4a81ff7
--- a/mailer.js
+++ b/mailer.js
@ -66,7 +66,11 @@ module.exports = function (log) {
    return linkAttributes(this.supportUrl)
  }

-  Mailer.prototype._initiatePasswordChange = function (email) {
+  Mailer.prototype._passwordResetLinkAttributes = function (email) {
+    return linkAttributes(this.createPasswordResetLink(email))
+  }
+
+  Mailer.prototype._passwordChangeLinkAttributes = function (email) {
    return linkAttributes(this.createPasswordChangeLink(email))
  }

@ -214,7 +218,7 @@ module.exports = function (log) {
        link: link,
        oneClickLink: oneClickLink,
        passwordChangeLink: this.createPasswordChangeLink(message.email),
-        passwordChangeLinkAttributes: this._initiatePasswordChange(message.email),
+        passwordChangeLinkAttributes: this._passwordChangeLinkAttributes(message.email),
        supportLinkAttributes: linkAttributes(this.supportUrl),
        supportUrl: this.supportUrl
      },
@ -288,7 +292,7 @@ module.exports = function (log) {
  }

  Mailer.prototype.passwordChangedEmail = function (message) {
-    var link = this.createPasswordChangeLink(message.email)
+    var link = this.createPasswordResetLink(message.email)

    return this.send({
      acceptLanguage: message.acceptLanguage,
@ -299,8 +303,8 @@ module.exports = function (log) {
      subject: gettext('Your Firefox Account password has been changed'),
      template: 'passwordChangedEmail',
      templateValues: {
-        passwordChangeLinkAttributes: this._initiatePasswordChange(message.email),
        resetLink: link,
+        resetLinkAttributes: this._passwordResetLinkAttributes(message.email),
        signInUrl: this.signInUrl,
        supportLinkAttributes: this._supportLinkAttributes(),
        supportUrl: this.supportUrl
@ -322,16 +326,16 @@ module.exports = function (log) {
      template: 'passwordResetEmail',
      templateValues: {
        resetLink: link,
+        resetLinkAttributes: this._passwordResetLinkAttributes(message.email),
        supportUrl: this.supportUrl,
-        supportLinkAttributes: this._supportLinkAttributes(),
-        resetPasswordUrl: this.signInUrl
+        supportLinkAttributes: this._supportLinkAttributes()
      },
      uid: message.uid
    })
  }

  Mailer.prototype.passwordResetRequiredEmail = function (message) {
-    var link = this.createPasswordResetLink(message.email, { reset_password_confirm: false })
+    var link = this.createPasswordResetLink(message.email)

    return this.send({
      acceptLanguage: message.acceptLanguage,
@ -368,7 +372,7 @@ module.exports = function (log) {
      template: 'newDeviceLoginEmail',
      templateValues: {
        device: this._formatUserAgentInfo(message),
-        passwordChangeLinkAttributes: this._initiatePasswordChange(message.email),
+        passwordChangeLinkAttributes: this._passwordChangeLinkAttributes(message.email),
        resetLink: link,
        supportLinkAttributes: this._supportLinkAttributes(),
        supportUrl: this.supportUrl,
@ -408,7 +412,7 @@ module.exports = function (log) {
  Mailer.prototype.suspiciousLocationEmail = function (message) {
    log.trace({ op: 'mailer.suspiciousLocationEmail', email: message.email, uid: message.uid })

-    var link = this.createPasswordResetLink(message.email, { reset_password_confirm: false })
+    var link = this.createPasswordResetLink(message.email)

    // the helper function `t` references `this.translator`. Because of
    // the way Handlebars `each` loops work, a translator instance must be
@ -480,13 +484,8 @@ module.exports = function (log) {
    })
  }

-  Mailer.prototype.createPasswordResetLink = function (email, extraQueryParams) {
-    var queryParams = { email: email }
-    extraQueryParams = extraQueryParams || {}
-
-    for (var key in extraQueryParams) {
-      queryParams[key] = extraQueryParams[key]
-    }
+  Mailer.prototype.createPasswordResetLink = function (email) {
+    var queryParams = { email: email, reset_password_confirm: false }

    return this.initiatePasswordResetUrl + '?' + qs.stringify(queryParams)
  }
--- a/partials/password_changed.html
+++ b/partials/password_changed.html
@ -1,4 +1,21 @@
-<% extends "partials/base/base_text_only.html" %>
+<% extends "partials/base/base.html" %>

-<% block heading %>{{t "Password changed successfully"}}<% endblock %>
-<% block text_primary %>{{t "Your Firefox Account password has been successfully changed." }}<% endblock %>
+<% block content %>
+<!--Header Area-->
+<tr style="page-break-before: always">
+  <td valign="top">
+    <h1 style="font-family: sans-serif; font-weight: normal; margin: 0 0 24px 0; text-align: center;">{{t "Password changed successfully"}}</h1>
+    <p class="primary" style="font-family: sans-serif; font-size: 14px; font-weight: normal; margin: 0 0 12px 0; text-align: center;">{{t "Your Firefox Account password has been successfully changed." }}</p>
+  </td>
+</tr>
+
+<!--Button Area-->
+<tr style="page-break-before: always">
+  <td border="0" cellpadding="0" cellspacing="0" height="100%" width="100%">
+    <br/>
+    <p class="secondary" style="font-family: sans-serif; font-weight: normal; margin: 0 0 12px 0; text-align: center; color: #8A9BA8; font-size: 11px; line-height: 13px; width: 310px !important; word-wrap:break-word">
+      {{{t "This is an automated email; if you did not authorize this action, then <a %(resetLinkAttributes)s>please reset your password</a>." }}} {{{t "For more information, please visit <a %(supportLinkAttributes)s>Mozilla Support</a>."}}}
+    </p>
+  </td>
+</tr>
+<% endblock %>
--- a/partials/password_reset.html
+++ b/partials/password_reset.html
@ -5,7 +5,7 @@
 <tr style="page-break-before: always">
    <td valign="top">
        <h3 style="font-family: sans-serif; font-weight: normal; margin: 0 0 24px 0; text-align: center;">{{t "Your password has been reset"}}</h3>
-        <p class="primary" style="font-family: sans-serif; font-size: 14px; font-weight: normal; margin: 0 0 12px 0; text-align: center;">{{{t "Your Firefox Account password has changed. If you did not change it, please <a href='%(resetLink)s'>reset your password</a> now." }}}</p>
+        <p class="primary" style="font-family: sans-serif; font-size: 14px; font-weight: normal; margin: 0 0 12px 0; text-align: center;">{{{t "Your Firefox Account password has changed. If you did not change it, please <a %(resetLinkAttributes)s>reset your password</a> now." }}}</p>
    </td>
 </tr>

--- a/templates/password_changed.html
+++ b/templates/password_changed.html
@ -24,11 +24,12 @@
  </td>
 </tr>

+<!--Button Area-->
 <tr style="page-break-before: always">
  <td border="0" cellpadding="0" cellspacing="0" height="100%" width="100%">
    <br/>
    <p class="secondary" style="font-family: sans-serif; font-weight: normal; margin: 0 0 12px 0; text-align: center; color: #8A9BA8; font-size: 11px; line-height: 13px; width: 310px !important; word-wrap:break-word">
-      {{{t "This is an automated email; if you did not authorize this action, then <a %(passwordChangeLinkAttributes)s>please change your password.</a>" }}} {{{t "For more information, please visit <a %(supportLinkAttributes)s>Mozilla Support</a>."}}}
+      {{{t "This is an automated email; if you did not authorize this action, then <a %(resetLinkAttributes)s>please reset your password</a>." }}} {{{t "For more information, please visit <a %(supportLinkAttributes)s>Mozilla Support</a>."}}}
    </p>
  </td>
 </tr>
--- a/templates/password_changed.txt
+++ b/templates/password_changed.txt
@ -1,7 +1,7 @@
 {{t "Password changed successfully" }}
 {{t "Your Firefox Account password has been successfully changed." }}

-{{t "This is an automated email; if you didn’t change the password to your Firefox Account, you should reset it immediately at %(resetLink)s." }}
+{{{t "This is an automated email; if you didn’t change the password to your Firefox Account, you should reset it immediately at %(resetLink)s." }}}
 {{t "For more information, please visit %(supportUrl)s" }}

 Mozilla. 331 E Evelyn Ave, Mountain View, CA 94041
--- a/templates/password_reset.html
+++ b/templates/password_reset.html
@ -20,7 +20,7 @@
 <tr style="page-break-before: always">
    <td valign="top">
        <h3 style="font-family: sans-serif; font-weight: normal; margin: 0 0 24px 0; text-align: center;">{{t "Your password has been reset"}}</h3>
-        <p class="primary" style="font-family: sans-serif; font-size: 14px; font-weight: normal; margin: 0 0 12px 0; text-align: center;">{{{t "Your Firefox Account password has changed. If you did not change it, please <a href='%(resetLink)s'>reset your password</a> now." }}}</p>
+        <p class="primary" style="font-family: sans-serif; font-size: 14px; font-weight: normal; margin: 0 0 12px 0; text-align: center;">{{{t "Your Firefox Account password has changed. If you did not change it, please <a %(resetLinkAttributes)s>reset your password</a> now." }}}</p>
    </td>
 </tr>

--- a/templates/password_reset.txt
+++ b/templates/password_reset.txt
@ -1,9 +1,7 @@
 {{t "Password reset successfully" }}
 {{t "Your Firefox Account password has been successfully reset." }}

-{{{t "Your Firefox Account password has changed. If you did not change it, please <a href='%(resetLink)s'>reset your password</a> now." }}}
-
-{{t "This is an automated email; if you didn’t reset the password to your Firefox Account, you should reset it immediately at %(resetLink)s." }}
+{{{t "This is an automated email; if you didn’t reset the password to your Firefox Account, you should reset it immediately at %(resetLink)s." }}}
 {{t "For more information, please visit %(supportUrl)s" }}

 Mozilla. 331 E Evelyn Ave, Mountain View, CA 94041
--- a/test/local/mailer_tests.js
+++ b/test/local/mailer_tests.js
@ -38,7 +38,9 @@ var typesWithSupportLinks = [
  'verifyEmail'
 ]

-var typesContainConfirmlessPasswordResetLinks = [
+var typesContainPasswordResetLinks = [
+  'passwordChangedEmail',
+  'passwordResetEmail',
  'passwordResetRequiredEmail',
  'suspiciousLocationEmail'
 ]
@ -90,15 +92,15 @@ P.all(
          )
        }

-        if (includes(typesContainConfirmlessPasswordResetLinks, type)) {
-          var confirmlessResetPasswordLink = mailer.createPasswordResetLink(message.email, { reset_password_confirm: false })
+        if (includes(typesContainPasswordResetLinks, type)) {
+          var resetPasswordLink = mailer.createPasswordResetLink(message.email)

          test(
            'reset password link is in email template output for ' + type,
            function (t) {
              mailer.mailer.sendMail = function (emailConfig) {
-                t.ok(includes(emailConfig.html, confirmlessResetPasswordLink))
-                t.ok(includes(emailConfig.text, confirmlessResetPasswordLink))
+                t.ok(includes(emailConfig.html, resetPasswordLink))
+                t.ok(includes(emailConfig.text, resetPasswordLink))
                t.end()
              }
              mailer[type](message)