From b179390e3333c5821b4cf2241b271e5b125d6129 Mon Sep 17 00:00:00 2001
From: Ben Adida <ben@adida.net>
Date: Tue, 10 Apr 2012 17:12:52 -0700
Subject: [PATCH] fixed check for site parameter, it should be an origin not a
domain, and fixed all tests accordingly. Was careful not to screw up the
verifier tests that are testing for old parameters to the verifier.
---
lib/sanitize.js | 12 ++++++++++--
lib/validate.js | 7 +++++--
lib/wsapi/stage_email.js | 2 +-
lib/wsapi/stage_user.js | 2 +-
tests/add-email-with-assertion-test.js | 2 +-
tests/cert-emails-test.js | 2 +-
tests/email-throttling-test.js | 8 ++++----
tests/forgotten-email-test.js | 6 +++---
tests/list-emails-wsapi-test.js | 2 +-
tests/no-cookie-test.js | 2 +-
tests/password-bcrypt-update-test.js | 2 +-
tests/password-length-test.js | 2 +-
tests/password-update-test.js | 2 +-
tests/primary-then-secondary-test.js | 4 ++--
tests/registration-status-wsapi-test.js | 4 ++--
tests/stalled-mysql-test.js | 6 +++---
tests/verifier-test.js | 2 +-
17 files changed, 39 insertions(+), 28 deletions(-)
diff --git a/lib/sanitize.js b/lib/sanitize.js
index dd02b5d22..78d905bb2 100644
--- a/lib/sanitize.js
+++ b/lib/sanitize.js
@@ -28,9 +28,17 @@ module.exports = function (value) {
throw "not a valid domain";
}
};
-
+
+ var isOrigin = function() {
+ // allow single hostnames, e.g. localhost
+ if (!value.match(/^https?:\/\/[a-z\d-]+(\.[a-z\d-]+)*(:\d+)?$/i)) {
+ throw "not a valid origin";
+ }
+ };
+
return {
isEmail: isEmail,
- isDomain: isDomain
+ isDomain: isDomain,
+ isOrigin: isOrigin
};
};
diff --git a/lib/validate.js b/lib/validate.js
index ebe808dd3..7eb2a6f04 100644
--- a/lib/validate.js
+++ b/lib/validate.js
@@ -31,11 +31,14 @@ module.exports = function (params) {
throw k;
}
});
- next();
} catch(e) {
var msg = "missing '" + e + "' argument";
- logger.warn("bad request recieved: " + msg);
+ logger.warn("bad request received: " + msg);
return httputils.badRequest(resp, msg);
}
+
+ // this is called outside the try/catch because errors
+ // in the handling of the request should be caught separately
+ next();
};
};
diff --git a/lib/wsapi/stage_email.js b/lib/wsapi/stage_email.js
index 7d59d4924..76bca3b86 100644
--- a/lib/wsapi/stage_email.js
+++ b/lib/wsapi/stage_email.js
@@ -26,7 +26,7 @@ exports.process = function(req, res) {
// validate
// should do this one but it's failing for some reason
sanitize(req.body.email).isEmail();
- sanitize(req.body.site).isDomain();
+ sanitize(req.body.site).isOrigin();
db.lastStaged(req.body.email, function (err, last) {
if (err) return wsapi.databaseDown(res, err);
diff --git a/lib/wsapi/stage_user.js b/lib/wsapi/stage_user.js
index 7ff035f29..ca1a3318c 100644
--- a/lib/wsapi/stage_user.js
+++ b/lib/wsapi/stage_user.js
@@ -30,7 +30,7 @@ exports.process = function(req, resp) {
// validate
sanitize(req.body.email).isEmail();
- sanitize(req.body.site).isDomain();
+ sanitize(req.body.site).isOrigin();
db.lastStaged(req.body.email, function (err, last) {
if (err) return wsapi.databaseDown(resp, err);
diff --git a/tests/add-email-with-assertion-test.js b/tests/add-email-with-assertion-test.js
index ce85baeab..aca86c04c 100755
--- a/tests/add-email-with-assertion-test.js
+++ b/tests/add-email-with-assertion-test.js
@@ -112,7 +112,7 @@ suite.addBatch({
"stage an account": {
topic: wsapi.post('/wsapi/stage_user', {
email: TEST_FIRST_ACCT,
- site:'fakesite.com'
+ site:'http://fakesite.com:652'
}),
"works": function(err, r) {
assert.strictEqual(r.code, 200);
diff --git a/tests/cert-emails-test.js b/tests/cert-emails-test.js
index d58dea80a..e8b2ef5a3 100755
--- a/tests/cert-emails-test.js
+++ b/tests/cert-emails-test.js
@@ -35,7 +35,7 @@ suite.addBatch({
topic: wsapi.post('/wsapi/stage_user', {
email: 'syncer@somehost.com',
pubkey: 'fakekey',
- site:'fakesite.com'
+ site:'http://fakesite.com'
}),
"succeeds": function(err, r) {
assert.strictEqual(r.code, 200);
diff --git a/tests/email-throttling-test.js b/tests/email-throttling-test.js
index ba807a782..4cceed04d 100755
--- a/tests/email-throttling-test.js
+++ b/tests/email-throttling-test.js
@@ -24,7 +24,7 @@ suite.addBatch({
"staging a registration": {
topic: wsapi.post('/wsapi/stage_user', {
email: 'first@fakeemail.com',
- site:'fakesite.com'
+ site:'https://fakesite.com:443'
}),
"returns 200": function(err, r) {
assert.strictEqual(r.code, 200);
@@ -49,7 +49,7 @@ suite.addBatch({
"immediately staging another": {
topic: wsapi.post('/wsapi/stage_user', {
email: 'first@fakeemail.com',
- site:'fakesite.com'
+ site:'http://fakesite.com:80'
}),
"is throttled": function(err, r) {
assert.strictEqual(r.code, 429);
@@ -74,7 +74,7 @@ suite.addBatch({
"add a new email address to our account": {
topic: wsapi.post('/wsapi/stage_email', {
email: 'second@fakeemail.com',
- site:'fakesite.com'
+ site:'https://fakesite.com'
}),
"works": function(err, r) {
assert.strictEqual(r.code, 200);
@@ -99,7 +99,7 @@ suite.addBatch({
"re-adding that same new email address a second time": {
topic: wsapi.post('/wsapi/stage_email', {
email: 'second@fakeemail.com',
- site:'fakesite.com'
+ site:'http://fakesite.com'
}),
"is throttled with a 429": function(err, r) {
assert.strictEqual(r.code, 429);
diff --git a/tests/forgotten-email-test.js b/tests/forgotten-email-test.js
index ed0fbc8ce..7c4f875e6 100755
--- a/tests/forgotten-email-test.js
+++ b/tests/forgotten-email-test.js
@@ -25,7 +25,7 @@ suite.addBatch({
"staging an account": {
topic: wsapi.post('/wsapi/stage_user', {
email: 'first@fakeemail.com',
- site:'fakesite.com'
+ site:'http://localhost:123'
}),
"works": function(err, r) {
assert.strictEqual(r.code, 200);
@@ -74,7 +74,7 @@ suite.addBatch({
"add a new email address to our account": {
topic: wsapi.post('/wsapi/stage_email', {
email: 'second@fakeemail.com',
- site:'fakesite.com'
+ site:'https://fakesite.foobar.bizbaz.uk'
}),
"works": function(err, r) {
assert.strictEqual(r.code, 200);
@@ -137,7 +137,7 @@ suite.addBatch({
"re-stage first account": {
topic: wsapi.post('/wsapi/stage_user', {
email: 'first@fakeemail.com',
- site:'otherfakesite.com'
+ site:'https://otherfakesite.com'
}),
"works": function(err, r) {
assert.strictEqual(r.code, 200);
diff --git a/tests/list-emails-wsapi-test.js b/tests/list-emails-wsapi-test.js
index 7ed5a742a..09fa35775 100755
--- a/tests/list-emails-wsapi-test.js
+++ b/tests/list-emails-wsapi-test.js
@@ -27,7 +27,7 @@ suite.addBatch({
"stage an account": {
topic: wsapi.post('/wsapi/stage_user', {
email: 'syncer@somehost.com',
- site:'fakesite.com'
+ site:'https://foobar.fakesite.com'
}),
"works": function(err, r) {
assert.strictEqual(r.code, 200);
diff --git a/tests/no-cookie-test.js b/tests/no-cookie-test.js
index 150d92cd4..33a2db02f 100755
--- a/tests/no-cookie-test.js
+++ b/tests/no-cookie-test.js
@@ -27,7 +27,7 @@ suite.addBatch({
"start registration": {
topic: wsapi.post('/wsapi/stage_user', {
email: 'first@fakeemail.com',
- site:'fakesite.com'
+ site:'http://fakesite.com:123'
}),
"returns 200": function(err, r) {
assert.strictEqual(r.code, 200);
diff --git a/tests/password-bcrypt-update-test.js b/tests/password-bcrypt-update-test.js
index c403d6ef7..de9ea66d4 100755
--- a/tests/password-bcrypt-update-test.js
+++ b/tests/password-bcrypt-update-test.js
@@ -46,7 +46,7 @@ suite.addBatch({
"account staging": {
topic: wsapi.post('/wsapi/stage_user', {
email: TEST_EMAIL,
- site:'fakesite.com'
+ site:'https://fakesite.com'
}),
"works": function(err, r) {
assert.equal(r.code, 200);
diff --git a/tests/password-length-test.js b/tests/password-length-test.js
index 2c956b1a1..8988e94cd 100755
--- a/tests/password-length-test.js
+++ b/tests/password-length-test.js
@@ -44,7 +44,7 @@ suite.addBatch({
"account staging": {
topic: wsapi.post('/wsapi/stage_user', {
email: 'first@fakeemail.com',
- site:'fakesite.com'
+ site:'https://fakesite.com:123'
}),
"works": function(err, r) {
assert.equal(r.code, 200);
diff --git a/tests/password-update-test.js b/tests/password-update-test.js
index 9d484f56f..dcaeda0d5 100755
--- a/tests/password-update-test.js
+++ b/tests/password-update-test.js
@@ -34,7 +34,7 @@ suite.addBatch({
"account staging": {
topic: wsapi.post('/wsapi/stage_user', {
email: TEST_EMAIL,
- site: 'fakesite.com'
+ site: 'https://fakesite.com:123'
}),
"works": function(err, r) {
assert.equal(r.code, 200);
diff --git a/tests/primary-then-secondary-test.js b/tests/primary-then-secondary-test.js
index 6b47385a9..da0e7e510 100755
--- a/tests/primary-then-secondary-test.js
+++ b/tests/primary-then-secondary-test.js
@@ -81,7 +81,7 @@ suite.addBatch({
"add a new email address to our account": {
topic: wsapi.post('/wsapi/stage_email', {
email: SECONDARY_EMAIL,
- site:'fakesite.com'
+ site:'https://fakesite.com'
}),
"works": function(err, r) {
assert.strictEqual(r.code, 200);
@@ -146,7 +146,7 @@ suite.addBatch({
"add a new email address to our account": {
topic: wsapi.post('/wsapi/stage_email', {
email: SECOND_SECONDARY_EMAIL,
- site:'fakesite.com'
+ site:'http://fakesite.com:123'
}),
"works": function(err, r) {
assert.strictEqual(r.code, 200);
diff --git a/tests/registration-status-wsapi-test.js b/tests/registration-status-wsapi-test.js
index 394383c20..cac3a6050 100755
--- a/tests/registration-status-wsapi-test.js
+++ b/tests/registration-status-wsapi-test.js
@@ -47,7 +47,7 @@ suite.addBatch({
"start registration": {
topic: wsapi.post('/wsapi/stage_user', {
email: 'first@fakeemail.com',
- site:'fakesite.com'
+ site:'https://fakesite.com'
}),
"returns 200": function(err, r) {
assert.strictEqual(r.code, 200);
@@ -166,7 +166,7 @@ suite.addBatch({
"re-registering an existing email": {
topic: wsapi.post('/wsapi/stage_user', {
email: 'first@fakeemail.com',
- site:'secondfakesite.com'
+ site:'http://secondfakesite.com'
}),
"yields a HTTP 200": function (err, r) {
assert.strictEqual(r.code, 200);
diff --git a/tests/stalled-mysql-test.js b/tests/stalled-mysql-test.js
index cd40adba4..a5bba9507 100755
--- a/tests/stalled-mysql-test.js
+++ b/tests/stalled-mysql-test.js
@@ -146,7 +146,7 @@ suite.addBatch({
"stage_user": {
topic: wsapi.post('/wsapi/stage_user', {
email: 'bogus@bogus.edu',
- site: 'whatev.er'
+ site: 'https://whatev.er'
}),
"fails with 503": function(err, r) {
assert.strictEqual(r.code, 503);
@@ -175,7 +175,7 @@ suite.addBatch({
"account staging": {
topic: wsapi.post('/wsapi/stage_user', {
email: "stalltest@whatev.er",
- site: 'fakesite.com'
+ site: 'http://fakesite.com'
}),
"works": function(err, r) {
assert.equal(r.code, 200);
@@ -264,7 +264,7 @@ suite.addBatch({
"stage_email": {
topic: wsapi.post('/wsapi/stage_email', {
email: "test2@whatev.er",
- site: "foo.com"
+ site: "https://foo.com"
}),
"fails with 503": function(err, r) {
assert.strictEqual(r.code, 503);
diff --git a/tests/verifier-test.js b/tests/verifier-test.js
index b2a455dc0..d9b0589b3 100755
--- a/tests/verifier-test.js
+++ b/tests/verifier-test.js
@@ -41,7 +41,7 @@ suite.addBatch({
"account staging": {
topic: wsapi.post('/wsapi/stage_user', {
email: TEST_EMAIL,
- site: TEST_DOMAIN
+ site: TEST_ORIGIN
}),
"works": function(err, r) {
assert.equal(r.code, 200);
--
GitLab