Merge branch 'train-107'

CHANGELOG.md

@@ -1,3 +1,13 @@
+<a name="1.107.3"></a>
+## [1.107.3](https://github.com/mozilla/fxa-auth-server/compare/v1.107.2-private-b...v1.107.3) (2018-03-16)
+
+
+### Bug Fixes
+
+* **validators:** Normalize redirectTo url to avoid parsing edge-cases. (#71) r=@vladikoff ([bb17257](https://github.com/mozilla/fxa-auth-server/commit/bb17257)), closes [#71](https://github.com/mozilla/fxa-auth-server/issues/71)
+
+
+
 <a name="1.107.2"></a>
 ## [1.107.2](https://github.com/mozilla/fxa-auth-server/compare/v1.107.1...v1.107.2) (2018-03-15)
 

lib/routes/validators.js

@@ -131,16 +131,19 @@ module.exports.isValidEmailAddress = function(value) {
   return hasDot
 }
 
-module.exports.redirectTo = function (base) {
-  var redirectTo = isA.string().max(512)
-  if (! base) { return redirectTo }
-  var regex = new RegExp('(?:\\.|^)' + base.replace('.', '\\.') + '$')
+module.exports.redirectTo = function redirectTo(base) {
+  const redirectTo = isA.string().max(512)
+  let hostnameRegex = null
+  if (base) {
+    hostnameRegex = new RegExp('(?:\\.|^)' + base.replace('.', '\\.') + '$')
+  }
   redirectTo._tests.push(
     {
-      func: function(value, state, options) {
+      func: (value, state, options) => {
         if (value !== undefined && value !== null) {
-          if (module.exports.isValidUrl(value, regex)) {
-            return value
+          const normalizedValue = module.exports.isValidUrl(value, hostnameRegex)
+          if (normalizedValue) {
+            return normalizedValue
           }
         }
 
@@ -151,9 +154,19 @@ module.exports.redirectTo = function (base) {
   return redirectTo
 }
 
-module.exports.isValidUrl = function (redirect, regex) {
+module.exports.isValidUrl = function (redirect, hostnameRegex) {
   var parsed = url.parse(redirect)
-  return regex.test(parsed.hostname) && /^https?:$/.test(parsed.protocol)
+  if (hostnameRegex && ! hostnameRegex.test(parsed.hostname)) {
+    return false
+  }
+  if (! /^https?:$/.test(parsed.protocol)) {
+    return false
+  }
+  // Normalize to the full URL string as understood by node.
+  // This helps avoid edge-cases where the browser might parse the URL
+  // differently to the way that node parsed it.
+  // See e.g. https://bugzilla.mozilla.org/show_bug.cgi?id=1445927
+  return parsed.href
 }
 
 module.exports.verificationMethod = isA.string().valid(['email', 'email-2fa', 'email-captcha'])

npm-shrinkwrap.json

@@ -1,6 +1,6 @@
 {
   "name": "fxa-auth-server",
-  "version": "1.107.2",
+  "version": "1.107.3",
   "dependencies": {
     "acorn": {
       "version": "5.0.3",

package.json

@@ -1,6 +1,6 @@
 {
   "name": "fxa-auth-server",
-  "version": "1.107.2",
+  "version": "1.107.3",
   "description": "Firefox Accounts, an identity provider for Mozilla cloud services",
   "bin": {
     "fxa-auth": "./bin/key_server.js"

test/local/routes/validators.js

@@ -0,0 +1,97 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+'use strict'
+
+const assert = require('insist')
+
+const validators = require('../../../lib/routes/validators')
+
+describe('redirectTo() validator', () => {
+  describe('with no base hostname', () => {
+    const v = validators.redirectTo()
+
+    it('accepts a well-formed https:// URL', function () {
+      const res = v.validate('https://example.com/path')
+      assert.ok(! res.error)
+      assert.equal(res.value, 'https://example.com/path')
+    })
+
+    it('accepts a well-formed http:// URL', function () {
+      const res = v.validate('http://example.com/path')
+      assert.ok(! res.error)
+      assert.equal(res.value, 'http://example.com/path')
+    })
+
+    it('rejects a non-URL string', function () {
+      const res = v.validate('not a url')
+      assert.ok(res.error)
+      assert.equal(res.value, 'not a url')
+    })
+
+    it('rejects a non-http(s) URL', function () {
+      const res = v.validate('mailto:test@example.com')
+      assert.ok(res.error)
+      assert.equal(res.value, 'mailto:test@example.com')
+    })
+
+    it('normalizes trisky quoted chars in the hostname', function () {
+      const res = v.validate('https://example.com%2Eevil.com')
+      assert.ok(! res.error)
+      assert.equal(res.value, 'https://example.com/%2Eevil.com')
+    })
+  })
+
+  describe('with a base hostname', () => {
+    const v = validators.redirectTo('mozilla.com')
+
+    it('accepts a well-formed https:// URL at the base hostname', function () {
+      const res = v.validate('https://test.mozilla.com/path')
+      assert.ok(! res.error)
+      assert.equal(res.value, 'https://test.mozilla.com/path')
+    })
+
+    it('accepts a well-formed http:// URL at the base hostname', function () {
+      const res = v.validate('http://test.mozilla.com/path')
+      assert.ok(! res.error)
+      assert.equal(res.value, 'http://test.mozilla.com/path')
+    })
+
+    it('rejects a non-URL string', function () {
+      const res = v.validate('not a url')
+      assert.ok(res.error)
+      assert.equal(res.value, 'not a url')
+    })
+
+    it('rejects a non-http(s) URL at the base hostname', function () {
+      const res = v.validate('irc://irc.mozilla.com/#fxa')
+      assert.ok(res.error)
+      assert.equal(res.value, 'irc://irc.mozilla.com/#fxa')
+    })
+
+    it('rejects a well-formed https:// URL at a different hostname', function () {
+      const res = v.validate('https://test.example.com/path')
+      assert.ok(res.error)
+      assert.equal(res.value, 'https://test.example.com/path')
+    })
+
+    it('accepts a well-formed http:// URL at a different hostname', function () {
+      const res = v.validate('http://test.example.com/path')
+      assert.ok(res.error)
+      assert.equal(res.value, 'http://test.example.com/path')
+    })
+
+    it('normalizes trisky quoted chars after the base hostname', function () {
+      const res = v.validate('https://mozilla.com%2Eevil.com')
+      assert.ok(! res.error)
+      assert.equal(res.value, 'https://mozilla.com/%2Eevil.com')
+    })
+
+    it('rejects trisky quoted chars before the base hostname', function () {
+      const res = v.validate('https://evil.com%2Emozilla.com')
+      assert.ok(res.error)
+      assert.equal(res.value, 'https://evil.com%2Emozilla.com')
+    })
+  })
+})

test/remote/password_forgot_tests.js

@@ -217,7 +217,7 @@ describe('remote password forgot', function() {
       var password = 'something'
       var client = null
       var options = {
-        redirectTo: 'https://sync.' + config.smtp.redirectDomain,
+        redirectTo: 'https://sync.' + config.smtp.redirectDomain + '/',
         service: 'sync'
       }
       return Client.create(config.publicUrl, email, password, options)
@@ -350,7 +350,7 @@ describe('remote password forgot', function() {
     () => {
       var email = server.uniqueEmail()
       var options = {
-        redirectTo: 'https://sync.' + config.smtp.redirectDomain,
+        redirectTo: 'https://sync.' + config.smtp.redirectDomain + '/',
         serviceQuery: 'sync'
       }
       var client

test/remote/recovery_email_verify_tests.js

@@ -76,7 +76,7 @@ describe('remote recovery email verify', function() {
       var password = 'something'
       var client = null // eslint-disable-line no-unused-vars
       var options = {
-        redirectTo: 'https://sync.'  + config.smtp.redirectDomain,
+        redirectTo: 'https://sync.'  + config.smtp.redirectDomain + '/',
         service: 'sync'
       }
       return Client.create(config.publicUrl, email, password, options)