diff --git a/README.md b/README.md index d0e9251b0570eeffe0061d59a4c6fcce5c3ab71b..138ed58b729cd82ab8643accb475b40d11025efe 100644 --- a/README.md +++ b/README.md @@ -15,6 +15,7 @@ All of the servers here are based on node.js, and some number of 3rd party node * vows (>= 0.5.8) * bcrypt (>= 0.2.3) * ejs (>= 0.4.3) +* temp (>= 0.2.0) * express-csrf (>= 0.3.2) * uglify (>= 1.0.6) diff --git a/browserid/lib/db.js b/browserid/lib/db.js index fe244d07ec3bc9c282c072d8ee98ca8cf68c105b..482fad4c1d92afa2b94ae982959d540a6f880571 100644 --- a/browserid/lib/db.js +++ b/browserid/lib/db.js @@ -1,35 +1,44 @@ const sqlite = require('sqlite'), -path = require('path'), -bcrypt = require('bcrypt'); +path = require('path'); var VAR_DIR = path.join(path.dirname(__dirname), "var"); var db = new sqlite.Database(); -var dbPath = path.join(VAR_DIR, "authdb.sqlite"); + +// a configurable parameter if set immediately after require() of db.js +exports.dbPath = path.join(VAR_DIR, "authdb.sqlite"); var ready = false; var waiting = []; -db.open(dbPath, function (error) { - if (error) { - console.log("Couldn't open database: " + error); - throw error; - } - db.executeScript( - "CREATE TABLE IF NOT EXISTS users ( id INTEGER PRIMARY KEY, password TEXT );" + - "CREATE TABLE IF NOT EXISTS emails ( id INTEGER PRIMARY KEY, user INTEGER, address TEXT UNIQUE );" + - "CREATE TABLE IF NOT EXISTS keys ( id INTEGER PRIMARY KEY, email INTEGER, key TEXT, expires INTEGER )", - function (error) { - if (error) { - throw error; - } - ready = true; - waiting.forEach(function(f) { f() }); - waiting = []; - }); -}); +// async break allow database path to be configured by calling code +// a touch tricky cause client must set dbPath before releasing +// control of the runloop +setTimeout(function() { + db.open(exports.dbPath, function (error) { + if (error) { + console.log("Couldn't open database: " + error); + throw error; + } + db.executeScript( + "CREATE TABLE IF NOT EXISTS users ( id INTEGER PRIMARY KEY, password TEXT );" + + "CREATE TABLE IF NOT EXISTS emails ( id INTEGER PRIMARY KEY, user INTEGER, address TEXT UNIQUE );" + + "CREATE TABLE IF NOT EXISTS keys ( id INTEGER PRIMARY KEY, email INTEGER, key TEXT, expires INTEGER )", + function (error) { + if (error) { + throw error; + } + ready = true; + waiting.forEach(function(f) { f() }); + waiting = []; + }); + }); +}, 0); +// accepts a function that will be invoked once the database is ready for transactions. +// this hook is important to pause the rest of application startup until async database +// connection establishment is complete. exports.onReady = function(f) { setTimeout(function() { if (ready) f(); @@ -37,6 +46,8 @@ exports.onReady = function(f) { }, 0); }; +// XXX: g_staged and g_stagedEmails should be moved into persistent/fast storage. + // half created user accounts (pending email verification) // OR // half added emails (pending verification) @@ -83,15 +94,6 @@ function emailToUserID(email, cb) { }); } -exports.findByEmail = function(email) { - for (var i = 0; i < g_users.length; i++) { - for (var j = 0; j < g_users[i].emails.length; j++) { - if (email === g_users[i].emails[j]) return g_users[i]; - } - } - return undefined; -}; - exports.emailKnown = function(email, cb) { db.execute( "SELECT id FROM emails WHERE address = ?", @@ -101,6 +103,7 @@ exports.emailKnown = function(email, cb) { }); }; +// XXX: should be moved to async. exports.isStaged = function(email) { return g_stagedEmails.hasOwnProperty(email); }; @@ -114,7 +117,7 @@ function generateSecret() { return str; } -exports.addEmailToAccount = function(existing_email, email, pubkey, cb) { +function addEmailToAccount(existing_email, email, pubkey, cb) { emailToUserID(existing_email, function(userID) { if (userID == undefined) { cb("no such email: " + existing_email, undefined); @@ -187,6 +190,7 @@ exports.stageUser = function(obj) { }; /* takes an argument object including email, pass, and pubkey. */ +// XXX: change to async exports.stageEmail = function(existing_email, new_email, pubkey) { var secret = generateSecret(); // overwrite previously staged users @@ -200,7 +204,7 @@ exports.stageEmail = function(existing_email, new_email, pubkey) { return secret; }; -/* invoked when a user clicks on a verification URL in their email */ +/* invoked when a user clicks on a verification URL in their email */ exports.gotVerificationSecret = function(secret, cb) { if (!g_staged.hasOwnProperty(secret)) return cb("unknown secret"); @@ -241,7 +245,7 @@ exports.gotVerificationSecret = function(secret, cb) { } else if (o.type === 'add_email') { exports.emailKnown(o.email, function(known) { function addIt() { - exports.addEmailToAccount(o.existing_email, o.email, o.pubkey, cb); + addEmailToAccount(o.existing_email, o.email, o.pubkey, cb); } if (known) { exports.removeEmail(o.email, o.email, function (err) { @@ -257,22 +261,25 @@ exports.gotVerificationSecret = function(secret, cb) { } }; -exports.checkAuth = function(email, pass, cb) { +// check authentication credentials for a given email address. This will invoke the +// users callback with the authentication (password/hash/whatever - the database layer +// doesn't care). callback will be passed undefined if email cannot be found +exports.checkAuth = function(email, cb) { db.execute("SELECT users.password FROM emails, users WHERE users.id = emails.user AND emails.address = ?", [ email ], function (error, rows) { - cb(rows.length === 1 && bcrypt.compare_sync(pass, rows[0].password)); - }); -}; - -exports.checkAuthHash = function(email, hash, cb) { - db.execute("SELECT users.password FROM emails, users WHERE users.id = emails.user AND emails.address = ? AND users.password = ?", - [ email, hash ], - function (error, rows) { - cb(rows.length === 1); + cb(rows.length !== 1 ? undefined : rows[0].password); }); }; +function emailHasPubkey(email, pubkey, cb) { + db.execute( + 'SELECT keys.key FROM keys, emails WHERE emails.address = ? AND keys.email = emails.id AND keys.key = ?', + [ email, pubkey ], + function(err, rows) { + cb(rows.length === 1); + }); +} /* a high level operation that attempts to sync a client's view with that of the * server. email is the identity of the authenticated channel with the user, @@ -291,7 +298,7 @@ exports.getSyncResponse = function(email, identities, cb) { key_refresh: [ ] }; - // get the user id associated with this account + // get the user id associated with this account emailToUserID(email, function(userID) { if (userID === undefined) { cb("no such email: " + email); @@ -304,44 +311,58 @@ exports.getSyncResponse = function(email, identities, cb) { if (err) cb(err); else { var emails = [ ]; + var keysToCheck = [ ]; for (var i = 0; i < rows.length; i++) emails.push(rows[i].address); // #1 for (var e in identities) { if (emails.indexOf(e) == -1) respBody.unknown_emails.push(e); + else keysToCheck.push(e); } // #2 for (var e in emails) { e = emails[e]; if (!identities.hasOwnProperty(e)) respBody.key_refresh.push(e); + } - // #3 - // XXX todo - - cb(undefined, respBody); + // #3 -- yes, this is sub-optimal in terms of performance. when we + // move away from public keys this will be unnec. + if (keysToCheck.length) { + var checked = 0; + keysToCheck.forEach(function(e) { + emailHasPubkey(e, identities[e], function(v) { + checked++; + if (!v) respBody.key_refresh.push(e); + if (checked === keysToCheck.length) { + cb(undefined, respBody); + } + }); + }); + } else { + cb(undefined, respBody); + } } }); }); }; - +// get all public keys associated with an email address exports.pubkeysForEmail = function(identity, cb) { - db.execute('SELECT keys.key FROM keys, emails WHERE emails.address = ? AND keys.email = emails.id', - [ identity ], - function(err, rows) { - var keys = undefined; - if (!err && rows && rows.length) { - keys = [ ]; - for (var i = 0; i < rows.length; i++) keys.push(rows[i].key); - } - cb(keys); - }); + db.execute( + 'SELECT keys.key FROM keys, emails WHERE emails.address = ? AND keys.email = emails.id', + [ identity ], + function(err, rows) { + var keys = undefined; + if (!err && rows && rows.length) { + keys = [ ]; + for (var i = 0; i < rows.length; i++) keys.push(rows[i].key); + } + cb(keys); + }); }; - -// FIXME: I'm not sure I'm using this data model properly exports.removeEmail = function(authenticated_email, email, cb) { // figure out the user, and remove Email only from addressed // linked to the authenticated email address @@ -367,4 +388,4 @@ exports.cancelAccount = function(authenticated_email, cb) { else cb(); }); }); -}; \ No newline at end of file +}; diff --git a/browserid/lib/wsapi.js b/browserid/lib/wsapi.js index 302c2f4b6b75ec22c1d9d93aa60e3d72ed5dd0cc..1291932c7b4a3789c6a78321a47e3473867553eb 100644 --- a/browserid/lib/wsapi.js +++ b/browserid/lib/wsapi.js @@ -18,7 +18,7 @@ function checkParams(params) { } else { params_in_request = req.query; } - + try { params.forEach(function(k) { if (!params_in_request.hasOwnProperty(k) || typeof params_in_request[k] !== 'string') { @@ -35,8 +35,6 @@ function checkParams(params) { function isAuthed(req) { var result= (req.session && typeof req.session.authenticatedUser === 'string'); - if (!result && req.session) - console.log("AUTH" + req.session.authenticatedUser); return result; } @@ -59,26 +57,26 @@ function setup(app) { httputils.jsonResponse(resp, known); }); }); - + /* First half of account creation. Stages a user account for creation. * this involves creating a secret url that must be delivered to the * user via their claimed email address. Upon timeout expiry OR clickthrough * the staged user account transitions to a valid user account */ app.post('/wsapi/stage_user', checkParams([ "email", "pass", "pubkey", "site" ]), function(req, resp) { - + // bcrypt the password // we should be cloning this object here. var stageParams = req.body; stageParams['hash'] = bcrypt.encrypt_sync(stageParams.pass, bcrypt.gen_salt_sync(10)); - + try { // upon success, stage_user returns a secret (that'll get baked into a url // and given to the user), on failure it throws var secret = db.stageUser(stageParams); - + // store the email being registered in the session data if (!req.session) req.session = {}; - + // store inside the session the details of this pending verification req.session.pendingVerification = { email: stageParams.email, @@ -89,12 +87,12 @@ function setup(app) { // representation of a user's password will get thrust into an encrypted cookie // served over an encrypted (SSL) session. guten, yah. }; - + httputils.jsonResponse(resp, true); - + // let's now kick out a verification email! email.sendVerificationEmail(stageParams.email, stageParams.site, secret); - + } catch(e) { // we should differentiate tween' 400 and 500 here. httputils.badRequest(resp, e.toString()); @@ -109,14 +107,14 @@ function setup(app) { httputils.badRequest(resp, "api abuse: registration_status called without a pending email addition/verification"); return; } - + // Is the current session trying to add an email, or register a new one? if (req.session.pendingAddition) { // this is a pending email addition, it requires authentication if (!isAuthed(req, resp)) { return httputils.badRequest(resp, "requires authentication"); } - + // check if the currently authenticated user has the email stored under pendingAddition // in their acct. db.emailsBelongToSameAccount(req.session.pendingAddition, @@ -132,10 +130,10 @@ function setup(app) { } else { // this is a pending registration, let's check if the creds stored on the // session are good yet. - + var v = req.session.pendingVerification; - db.checkAuthHash(v.email, v.hash, function(authed) { - if (authed) { + db.checkAuth(v.email, function(hash) { + if (hash === v.hash) { delete req.session.pendingVerification; req.session.authenticatedUser = v.email; httputils.jsonResponse(resp, "complete"); @@ -145,29 +143,31 @@ function setup(app) { }); } }); - - + + app.post('/wsapi/authenticate_user', checkParams(["email", "pass"]), function(req, resp) { - db.checkAuth(req.body.email, req.body.pass, function(rv) { - if (rv) { + db.checkAuth(req.body.email, function(hash) { + var success = bcrypt.compare_sync(req.body.pass, hash); + + if (success) { if (!req.session) req.session = {}; req.session.authenticatedUser = req.body.email; } - httputils.jsonResponse(resp, rv); + httputils.jsonResponse(resp, success); }); }); - + app.post('/wsapi/add_email', checkAuthed, checkParams(["email", "pubkey", "site"]), function (req, resp) { try { // upon success, stage_user returns a secret (that'll get baked into a url // and given to the user), on failure it throws var secret = db.stageEmail(req.session.authenticatedUser, req.body.email, req.body.pubkey); - + // store the email being added in session data req.session.pendingAddition = req.body.email; - + httputils.jsonResponse(resp, true); - + // let's now kick out a verification email! email.sendVerificationEmail(req.body.email, req.body.site, secret); } catch(e) { @@ -178,7 +178,7 @@ function setup(app) { app.post('/wsapi/remove_email', checkAuthed, checkParams(["email"]), function(req, resp) { var email = req.body.email; - + db.removeEmail(req.session.authenticatedUser, email, function(error) { if (error) { console.log("error removing email " + email); diff --git a/browserid/tests/db-test.js b/browserid/tests/db-test.js new file mode 100755 index 0000000000000000000000000000000000000000..bd7afd9145bf30bb15b9d2d0f9c6a645b1c353be --- /dev/null +++ b/browserid/tests/db-test.js @@ -0,0 +1,375 @@ +#!/usr/bin/env node + +const +assert = require('assert'), +vows = require('vows'), +db = require('../lib/db.js'), +temp = require('temp'), +fs = require('fs'), +path = require('path'); + +var suite = vows.describe('db'); +// disable vows (often flakey?) async error behavior +suite.options.error = false; + +db.dbPath = temp.path({suffix: '.sqlite'}); + +suite.addBatch({ + "waiting for the database to become ready": { + topic: function() { + var cb = this.callback; + db.onReady(function() { cb(true) }); + }, + "the database is ready": function(r) { + assert.strictEqual(r, true); + } + } +}); + +// caching of secrets between test batches. +var secret = undefined; + +suite.addBatch({ + "an email address is not reported as staged before it is": { + topic: function() { + return db.isStaged('lloyd@nowhe.re'); + }, + "isStaged returns false": function (r) { + assert.strictEqual(r, false); + } + }, + "an email address is not reported as known before it is": { + topic: function() { + db.emailKnown('lloyd@nowhe.re', this.callback); + }, + "emailKnown returns false": function (r) { + assert.strictEqual(r, false); + } + } +}); + +suite.addBatch({ + "stage a user for creation pending verification": { + topic: function() { + return secret = db.stageUser({ + email: 'lloyd@nowhe.re', + pubkey: 'fakepubkey', + hash: 'fakepasswordhash' + }); + }, + "staging returns a valid secret": function(r) { + assert.isString(secret); + assert.strictEqual(secret.length, 48); + } + } +}); + +suite.addBatch({ + "an email address is reported": { + topic: function() { + return db.isStaged('lloyd@nowhe.re'); + }, + " as staged after it is": function (r) { + assert.strictEqual(r, true); + } + }, + "an email address is not reported": { + topic: function() { + db.emailKnown('lloyd@nowhe.re', this.callback); + }, + " as known when it is only staged": function (r) { + assert.strictEqual(r, false); + } + } +}); + +suite.addBatch({ + "upon receipt of a secret": { + topic: function() { + db.gotVerificationSecret(secret, this.callback); + }, + "gotVerificationSecret completes without error": function (r) { + assert.strictEqual(r, undefined); + } + } +}); + +suite.addBatch({ + "an email address is not reported": { + topic: function() { + return db.isStaged('lloyd@nowhe.re'); + }, + "as staged immediately after its verified": function (r) { + assert.strictEqual(r, false); + } + }, + "an email address is known": { + topic: function() { + db.emailKnown('lloyd@nowhe.re', this.callback); + }, + "when it is": function (r) { + assert.strictEqual(r, true); + } + } +}); + +suite.addBatch({ + "adding keys to email": { + topic: function() { + db.addKeyToEmail('lloyd@nowhe.re', 'lloyd@nowhe.re', 'fakepubkey2', this.callback); + }, + "works": function(r) { + assert.isUndefined(r); + } + } +}); + +suite.addBatch({ + "adding multiple keys to email": { + topic: function() { + db.addKeyToEmail('lloyd@nowhe.re', 'lloyd@nowhe.re', 'fakepubkey3', this.callback); + }, + "works too": function(r) { + assert.isUndefined(r); + } + } +}); + +suite.addBatch({ + "pubkeysForEmail": { + topic: function() { + db.pubkeysForEmail('lloyd@nowhe.re', this.callback); + }, + "returns all public keys properly": function(r) { + assert.isArray(r); + assert.strictEqual(r.length, 3); + } + } +}); + +suite.addBatch({ + "checkAuth returns": { + topic: function() { + db.checkAuth('lloyd@nowhe.re', this.callback); + }, + "the correct password": function(r) { + assert.strictEqual(r, "fakepasswordhash"); + } + } +}); + +suite.addBatch({ + "staging an email": { + topic: function() { + return db.stageEmail('lloyd@nowhe.re', 'lloyd@somewhe.re', 'fakepubkey4'); + }, + "yields a valid secret": function(secret) { + assert.isString(secret); + assert.strictEqual(secret.length, 48); + }, + "makes email addr via isStaged": { + topic: function() { return db.isStaged('lloyd@somewhe.re'); }, + "visible": function(r) { assert.isTrue(r); } + }, + "and verifying it": { + topic: function(secret) { + db.gotVerificationSecret(secret, this.callback); + }, + "returns no error": function(r) { + assert.isUndefined(r); + }, + "makes email addr via knownEmail": { + topic: function() { db.emailKnown('lloyd@somewhe.re', this.callback); }, + "visible": function(r) { assert.isTrue(r); } + }, + "makes email addr via isStaged": { + topic: function() { return db.isStaged('lloyd@somewhe.re'); }, + "not visible": function(r) { assert.isFalse(r); } + } + } + } +}); + +// exports.emailsBelongToSameAccount +suite.addBatch({ + "emails do belong to the same account": { + "is true": { + topic: function() { + db.emailsBelongToSameAccount('lloyd@nowhe.re', 'lloyd@somewhe.re', this.callback); + }, + "when they do": function(r) { + assert.isTrue(r); + } + }, + "is false": { + topic: function() { + db.emailsBelongToSameAccount('lloyd@anywhe.re', 'lloyd@somewhe.re', this.callback); + }, + "when they don't": function(r) { + assert.isFalse(r); + } + } + } +}); + +// exports.getSyncResponse +suite.addBatch({ + "sync responses": { + "are empty": { + topic: function() { + db.getSyncResponse('lloyd@nowhe.re', + { + 'lloyd@nowhe.re': 'fakepubkey', + 'lloyd@somewhe.re': 'fakepubkey4' + }, + this.callback); + }, + "when everything is in sync": function (err, resp) { + assert.isUndefined(err); + assert.isArray(resp.unknown_emails); + assert.isArray(resp.key_refresh); + assert.strictEqual(resp.unknown_emails.length, 0); + assert.strictEqual(resp.key_refresh.length, 0); + } + }, + "handles client unknown emails": { + topic: function() { + db.getSyncResponse('lloyd@nowhe.re', + { + 'lloyd@nowhe.re': 'fakepubkey' + }, + this.callback); + }, + "by returning them in the key_refresh list": function (err, resp) { + assert.isUndefined(err); + assert.isArray(resp.unknown_emails); + assert.isArray(resp.key_refresh); + assert.strictEqual(resp.unknown_emails.length, 0); + assert.strictEqual(resp.key_refresh.length, 1); + assert.strictEqual(resp.key_refresh[0], 'lloyd@somewhe.re'); + } + }, + "handles server unknown emails": { + topic: function() { + db.getSyncResponse('lloyd@nowhe.re', + { + 'lloyd@nowhe.re': 'fakepubkey', + 'lloyd@somewhe.re': 'fakepubkey4', + 'lloyd@anywhe.re': 'nofakepubkey', + }, + this.callback); + }, + "by returning them in the unknown_emails list": function (err, resp) { + assert.isUndefined(err); + assert.isArray(resp.unknown_emails); + assert.strictEqual(resp.unknown_emails.length, 1); + assert.strictEqual(resp.unknown_emails[0], 'lloyd@anywhe.re'); + assert.isArray(resp.key_refresh); + assert.strictEqual(resp.key_refresh.length, 0); + } + }, + "handles server unknown keys": { + topic: function() { + db.getSyncResponse('lloyd@nowhe.re', + { + 'lloyd@nowhe.re': 'fakepubkeyINVALID', + 'lloyd@somewhe.re': 'fakepubkey4' + }, + this.callback); + }, + "by returning them in the key_refresh list": function (err, resp) { + assert.isUndefined(err); + assert.isArray(resp.unknown_emails); + assert.strictEqual(resp.unknown_emails.length, 0); + assert.isArray(resp.key_refresh); + assert.strictEqual(resp.key_refresh.length, 1); + assert.strictEqual(resp.key_refresh[0], 'lloyd@nowhe.re'); + } + }, + "handle more than one case at a time": { + topic: function() { + db.getSyncResponse('lloyd@nowhe.re', + { + 'lloyd@somewhe.re': 'fakepubkeyINVALID', + 'lloyd@anywhe.re': 'notreally' + }, + this.callback); + }, + "when everything is outta sync": function (err, resp) { + assert.isUndefined(err); + assert.isArray(resp.unknown_emails); + assert.strictEqual(resp.unknown_emails.length, 1); + assert.strictEqual(resp.unknown_emails[0], 'lloyd@anywhe.re'); + + assert.isArray(resp.key_refresh); + assert.strictEqual(resp.key_refresh.length, 2); + assert.strictEqual(resp.key_refresh[0], 'lloyd@nowhe.re'); + assert.strictEqual(resp.key_refresh[1], 'lloyd@somewhe.re'); + } + } + } +}); + +suite.addBatch({ + "removing an existing email": { + topic: function() { + db.removeEmail("lloyd@somewhe.re", "lloyd@nowhe.re", this.callback); + }, + "returns no error": function(r) { + assert.isUndefined(r); + }, + "causes emailKnown": { + topic: function() { + db.emailKnown('lloyd@nowhe.re', this.callback); + }, + "to return false": function (r) { + assert.strictEqual(r, false); + } + } + } +}); + +suite.addBatch({ + "canceling an account": { + topic: function() { + db.cancelAccount("lloyd@somewhe.re", this.callback); + }, + "returns no error": function(r) { + assert.isUndefined(r); + }, + "causes emailKnown": { + topic: function() { + db.emailKnown('lloyd@somewhe.re', this.callback); + }, + "to return false": function (r) { + assert.strictEqual(r, false); + } + } + } +}); + +// exports.cancelAccount +// exports.removeEmail + +suite.addBatch({ + "remove the database file": { + topic: function() { + fs.unlink(db.dbPath, this.callback); + }, + "and unlink should not error": function(err) { + assert.isNull(err); + }, + "and the file": { + topic: function() { + path.exists(db.dbPath, this.callback); + }, + "should be missing": function(r) { + assert.isFalse(r); + } + } + } +}); + +// run or export the suite. +if (process.argv[1] === __filename) suite.run(); +else suite.export(module); diff --git a/browserid/tests/lib/wsapi.js b/browserid/tests/lib/wsapi.js index f6ba8b174b764d43025c29580e51924b55d0dfe4..dc3a9c93c2cdc720ddab5f156984b66af2f0fb15 100644 --- a/browserid/tests/lib/wsapi.js +++ b/browserid/tests/lib/wsapi.js @@ -1,5 +1,6 @@ -const http = require('http'), - querystring = require('querystring'); +const +http = require('http'), +querystring = require('querystring'); // wsapi abstractions trivial cookie jar var cookieJar = {};