diff --git a/db-server/index.js b/db-server/index.js index 293a5c08..23a10ada 100644 --- a/db-server/index.js +++ b/db-server/index.js @@ -225,6 +225,16 @@ function createServer(db) { op(req => db.consumeSigninCode(req.params.code)) ) + api.post('/account/:id/recoveryCodes', + op((req) => { + return db.replaceRecoveryCodes(req.params.id, req.body.count) + }) + ) + + api.post('/account/:id/recoveryCodes/:code', + op((req) => db.consumeRecoveryCode(req.params.id, req.params.code)) + ) + api.get( '/', function (req, res, next) { diff --git a/db-server/test/backend/db_tests.js b/db-server/test/backend/db_tests.js index e03896cd..feb7445f 100644 --- a/db-server/test/backend/db_tests.js +++ b/db-server/test/backend/db_tests.js @@ -1985,6 +1985,103 @@ module.exports = function (config, DB) { }) }) + describe('recovery codes', () => { + let account + beforeEach(() => { + account = createAccount() + account.emailVerified = true + return db.createAccount(account.uid, account) + }) + + it('should fail to generate for unknown user', () => { + return db.replaceRecoveryCodes(hex16(), 2) + .then(assert.fail, (err) => { + assert.equal(err.errno, 116, 'correct errno, not found') + }) + }) + + const codeLengthTest = [0, 4, 8] + codeLengthTest.forEach((num) => { + it('should generate ' + num + ' recovery codes', () => { + return db.replaceRecoveryCodes(account.uid, num) + .then((codes) => { + assert.equal(codes.length, num, 'correct number of codes') + }, (err) => { + assert.equal(err.errno, 116, 'correct errno, not found') + }) + }) + }) + + it('should replace recovery codes', () => { + let firstCodes + return db.replaceRecoveryCodes(account.uid, 2) + .then((codes) => { + firstCodes = codes + assert.equal(firstCodes.length, 2, 'correct number of codes') + + return db.replaceRecoveryCodes(account.uid, 3) + }) + .then((codes) => { + assert.equal(codes.length, 3, 'correct number of codes') + assert.notDeepEqual(codes, firstCodes, 'codes are different') + }) + }) + + describe('should consume recovery codes', () => { + let recoveryCodes + beforeEach(() => { + return db.replaceRecoveryCodes(account.uid, 2) + .then((codes) => { + recoveryCodes = codes + assert.equal(recoveryCodes.length, 2, 'correct number of recovery codes') + }) + }) + + it('should fail to consume recovery code with unknown uid', () => { + return db.consumeRecoveryCode(hex16(), 'recoverycodez') + .then(assert.fail, (err) => { + assert.equal(err.errno, 116, 'correct errno, not found') + }) + }) + + it('should fail to consume recovery code with unknown code', () => { + return db.replaceRecoveryCodes(account.uid, 3) + .then(() => { + return db.consumeRecoveryCode(account.uid, 'notvalidcode') + .then(assert.fail, (err) => { + assert.equal(err.errno, 116, 'correct errno, unknown recovery code') + }) + }) + }) + + it('should fail to consume code twice', () => { + return db.consumeRecoveryCode(account.uid, recoveryCodes[0]) + .then((result) => { + assert.equal(result.remaining, 1, 'correct number of remaining codes') + + // Should fail to consume code twice + return db.consumeRecoveryCode(account.uid, recoveryCodes[0]) + .then(assert.fail, (err) => { + assert.equal(err.errno, 116, 'correct errno, unknown recovery code') + }) + }) + }) + + it('should consume code', () => { + return db.consumeRecoveryCode(account.uid, recoveryCodes[0]) + .then((result) => { + assert.equal(result.remaining, 1, 'correct number of remaining codes') + + return db.consumeRecoveryCode(account.uid, recoveryCodes[1]) + .then((result) => { + assert.equal(result.remaining, 0, 'correct number of remaining codes') + }) + }) + }) + }) + }) + + after(() => db.close()) }) } diff --git a/db-server/test/backend/remote.js b/db-server/test/backend/remote.js index 48cd729f..59279f62 100644 --- a/db-server/test/backend/remote.js +++ b/db-server/test/backend/remote.js @@ -1627,7 +1627,7 @@ module.exports = function(cfg, makeServer) { .then((res) => respOkEmpty(res)) }) - it('set session verification method', () => { + it('set session verification method - totp-2fa', () => { const verifyOptions = { verificationMethod: 'totp-2fa', } @@ -1643,6 +1643,60 @@ module.exports = function(cfg, makeServer) { }) }) + it('set session verification method - recovery-code', () => { + const verifyOptions = { + verificationMethod: 'recovery-code', + } + return client.postThen('/tokens/' + user.sessionTokenId + '/verifyWithMethod', verifyOptions) + .then((res) => { + respOkEmpty(res) + return client.getThen('/sessionToken/' + user.sessionTokenId + '/device') + }) + .then((sessionToken) => { + sessionToken = sessionToken.obj + assert.equal(sessionToken.verificationMethod, 3, 'verificationMethod set') + assert.ok(sessionToken.verifiedAt, 'verifiedAt set') + }) + }) + }) + + describe('recovery codes', () => { + let user + beforeEach(() => { + user = fake.newUserDataHex() + return client.putThen('/account/' + user.accountId, user.account) + .then((r) => { + respOkEmpty(r) + }) + }) + + it('should generate new recovery codes', () => { + return client.postThen('/account/' + user.accountId + '/recoveryCodes', {count: 8}) + .then((res) => { + const codes = res.obj + assert.equal(codes.length, 8, 'correct number of codes') + }) + }) + + it('should fail to consume unknown recovery code', () => { + return client.postThen('/account/' + user.accountId + '/recoveryCodes/' + '12345678') + .then(assert.fail, (err) => { + testNotFound(err) + }) + }) + + it('should consume recovery code', () => { + return client.postThen('/account/' + user.accountId + '/recoveryCodes', {count: 8}) + .then((res) => { + const codes = res.obj + assert.equal(codes.length, 8, 'correct number of codes') + return client.postThen('/account/' + user.accountId + '/recoveryCodes/' + codes[0]) + }) + .then((res) => { + const result = res.obj + assert.equal(result.remaining, 7, 'correct number of remaining codes') + }) + }) }) after(() => server.close()) diff --git a/docs/API.md b/docs/API.md index dea79589..01f29701 100644 --- a/docs/API.md +++ b/docs/API.md @@ -100,6 +100,9 @@ The following datatypes are used throughout this document: * totpToken : `GET /totp/:id` * deleteTotpToken : `DEL /totp/:id` * updateTotpToken : `POST /totp/:id/update` +* Recovery codes: + * replaceRecoveryCodes : `POST /account/:id/recoveryCodes` + * consumeRecoveryCode : `POST /account/:id/recoveryCodes/:code` ## Ping : `GET /` @@ -2066,3 +2069,90 @@ Content-Length: 2 * Conditions: if something goes wrong on the server * Content-Type : `application/json` * Body : `{"code":"InternalError","message":"..."}` + +## replaceRecoveryCodes : `GET /account/:uid/recoveryCodes` + +Replaces a users current recovery codes with new ones. + +### Example + +``` +curl \ + -v \ + -X POST \ + -H "Content-Type: application/json" \ + -d '{ + "count" : 8 + }' + http://localhost:8000/account/1234567890ab/recoveryCodes +``` + +### Request + +* Method : `POST` +* Path : `/account//recoveryCodes` + * `uid` : hex +* + +### Response + +``` +HTTP/1.1 200 OK +Content-Type: application/json +Content-Length: 2 + +["code1", "code2"] +``` + +* Status Code : `200 OK` + * Content-Type : `application/json` + * Body : `["code1", "code2"]` +* Status Code : `404 Not Found` + * Conditions: if no user found + * Content-Type : `application/json` +* Status Code : `500 Internal Server Error` + * Conditions: if something goes wrong on the server + * Content-Type : `application/json` + * Body : `{"code":"InternalError","message":"..."}` + +## consumeRecoveryCode : `POST /account/:uid/recoveryCodes/:code` + +Consumes a recovery code. + +### Example + +``` +curl \ + -v \ + -X POST \ + -H "Content-Type: application/json" \ + http://localhost:8000/account/1234567890ab/recoveryCodes/1123 +``` + +### Request + +* Method : `POST` +* Path : `/account//recoveryCodes/` + * `uid` : hex + * `code`: hex + +### Response + +``` +HTTP/1.1 200 OK +Content-Type: application/json +Content-Length: 2 + +{"remaining" : 1} +``` + +* Status Code : `200 OK` + * Content-Type : `application/json` + * Body : `{"remaining" : 1}` +* Status Code : `404 Not Found` + * Conditions: if no user found or code not found + * Content-Type : `application/json` +* Status Code : `500 Internal Server Error` + * Conditions: if something goes wrong on the server + * Content-Type : `application/json` + * Body : `{"code":"InternalError","message":"..."}` diff --git a/docs/DB_API.md b/docs/DB_API.md index 323b20cb..9c2e00f5 100644 --- a/docs/DB_API.md +++ b/docs/DB_API.md @@ -67,6 +67,9 @@ There are a number of methods that a DB storage backend should implement: * .totpToken(uid) * .deleteTotpToken(uid) * .updateTotpToken(uid, tokenData) +* Recovery codes + * .replaceRecoveryCodes(uid, count) + * .consumeRecoveryCode(uid, code) * General * .ping() * .close() @@ -814,76 +817,115 @@ Returns: ## createTotpToken(uid, sharedSecret, epoch) - Creates a new TOTP token for the user. +Creates a new TOTP token for the user. - Parameters: +Parameters: - * `uid` (Buffer16): - The uid of the owning account - * `sharedSecret` (string): - The shared secret used to generate TOTP code - * `epoch` (number): - The epoch used to generate TOTP code (default 0) +* `uid` (Buffer16): + The uid of the owning account +* `sharedSecret` (string): + The shared secret used to generate TOTP code +* `epoch` (number): + The epoch used to generate TOTP code (default 0) - Returns: +Returns: - * Resolves with: - * An empty object `{}` - * Rejects with: - * Any error from the underlying storage system (wrapped in `error.wrap()`) - * `error.duplicate()` if this user had a token already +* Resolves with: + * An empty object `{}` +* Rejects with: + * Any error from the underlying storage system (wrapped in `error.wrap()`) + * `error.duplicate()` if this user had a token already ## totpToken(uid) - Get's the TOTP token for the user. +Get's the TOTP token for the user. - Parameters: +Parameters: - * `uid` (Buffer16): - The uid of the owning account +* `uid` (Buffer16): + The uid of the owning account - Returns: +Returns: - * Resolves with: - * An object `{}` - * sharedSecret - * epoch - * Rejects with: - * Any error from the underlying storage system (wrapped in `error.wrap()`) - * `error.notFound()` if this user does not have a token +* Resolves with: + * An object `{}` + * sharedSecret + * epoch +* Rejects with: + * Any error from the underlying storage system (wrapped in `error.wrap()`) + * `error.notFound()` if this user does not have a token ## deleteTotpToken(uid) - Delete the TOTP token for the user. +Delete the TOTP token for the user. - Parameters: +Parameters: - * `uid` (Buffer16): - The uid of the owning account +* `uid` (Buffer16): + The uid of the owning account - Returns: +Returns: - * Resolves with: - * An empty object `{}` - * Rejects with: - * Any error from the underlying storage system (wrapped in `error.wrap()`) +* Resolves with: + * An empty object `{}` +* Rejects with: + * Any error from the underlying storage system (wrapped in `error.wrap()`) ## updateTotpToken(uid, tokenData) - Update the TOTP token for the user. +Update the TOTP token for the user. - Parameters: +Parameters: - * `uid` (Buffer16): - The uid of the owning account - * `tokenData` - * `verified`: Boolean whether TOTP token has been verified - * `enabled`: Boolean whether TOTP token is enabled +* `uid` (Buffer16): + The uid of the owning account +* `tokenData` + * `verified`: Boolean whether TOTP token has been verified + * `enabled`: Boolean whether TOTP token is enabled - Returns: +Returns: - * Resolves with: - * An empty object `{}` - * Rejects with: - * Any error from the underlying storage system (wrapped in `error.wrap()`) - * `error.notFound()` if this user does not have a token +* Resolves with: + * An empty object `{}` +* Rejects with: + * Any error from the underlying storage system (wrapped in `error.wrap()`) + * `error.notFound()` if this user does not have a token + +## replaceRecoveryCodes(uid, count) + +Replaces user's recovery codes with new ones. + +Parameters: + +* `uid` (Buffer16): + The uid of the owning account +* `count` (Integer): + The number of recovery codes to create + +Returns: + +* Resolves with: + * An array of recovery codes `[]` +* Rejects with: + * Any error from the underlying storage system (wrapped in `error.wrap()`) + * `error.notFound()` if this user not found + +## consumeRecoveryCode(uid, code) + +Consume a recovery code. + +Parameters: + +* `uid` (Buffer16): + The uid of the owning account +* `code` (String): + The code to consume + +Returns: + +* Resolves with: + * An array object containing + * `remaining` - number of recovery codes remaining +* Rejects with: + * Any error from the underlying storage system (wrapped in `error.wrap()`) + * `error.notFound()` if this user or code not found diff --git a/lib/db/mem.js b/lib/db/mem.js index 82cebf33..d0e189a0 100644 --- a/lib/db/mem.js +++ b/lib/db/mem.js @@ -26,6 +26,7 @@ var emailBounces = {} var emails = {} var signinCodes = {} const totpTokens = {} +const recoveryCodes = {} var DEVICE_FIELDS = [ 'sessionTokenId', @@ -1303,6 +1304,55 @@ module.exports = function (log, error) { }) } + Memory.prototype.replaceRecoveryCodes = function (uid, count) { + uid = uid.toString('hex') + return getAccountByUid(uid) + .then(() => { + return dbUtil.generateRecoveryCodes(count) + .then((codes) => { + recoveryCodes[uid] = codes.map((code) => { + return { + codeHash: dbUtil.createHashSha512(code) + } + }) + return codes + }) + }) + } + + Memory.prototype.consumeRecoveryCode = function (uid, code) { + uid = uid.toString('hex') + const codeHash = dbUtil.createHashSha512(code).toString('hex') + + return getAccountByUid(uid) + .then(() => { + const codes = recoveryCodes[uid] + + if (! codes) { + throw error.notFound() + } + + let foundCode, foundIndex + for (let i = 0; i < codes.length; i++) { + const code = codes[i] + if (codeHash === code.codeHash.toString('hex')) { + foundCode = code + foundIndex = i + break + } + } + + if (! foundCode) { + throw error.notFound() + } + + codes.splice(foundIndex, 1) + + return {createdAt: foundCode.createdAt, remaining: codes.length} + }) + } + + // UTILITY FUNCTIONS Memory.prototype.ping = function () { diff --git a/lib/db/mysql.js b/lib/db/mysql.js index a2f5ea76..3f2cd2f3 100644 --- a/lib/db/mysql.js +++ b/lib/db/mysql.js @@ -1413,5 +1413,62 @@ module.exports = function (log, error) { }) } + const DELETE_RECOVERY_CODES = 'CALL deleteRecoveryCodes_1(?)' + const INSERT_RECOVERY_CODE = 'CALL createRecoveryCode_1(?, ?)' + MySql.prototype.replaceRecoveryCodes = function (uid, count) { + + // Because of the hashing requirements the process of replacing + // recovery codes is done is two separate procedures. First one + // deletes all current codes and the second one inserts the + // hashed randomly generated codes. + return dbUtil.generateRecoveryCodes(count) + .then((codeList) => { + return this.read(DELETE_RECOVERY_CODES, [uid]) + .then(() => { + if (codeList <= 0) { + return P.resolve([]) + } + + const queries = [] + codeList.forEach((code) => { + queries.push({ + sql: INSERT_RECOVERY_CODE, + params: [uid, dbUtil.createHashSha512(code)] + }) + }) + + return this.writeMultiple(queries) + }) + .then(() => { + return P.resolve(codeList) + }) + .catch((err) => { + if (err.errno === 1643) { + throw error.notFound() + } + + throw err + }) + + }) + } + + const CONSUME_RECOVERY_CODE = 'CALL consumeRecoveryCode_1(?, ?)' + MySql.prototype.consumeRecoveryCode = function (uid, code) { + return this.readFirstResult(CONSUME_RECOVERY_CODE, [uid, dbUtil.createHashSha512(code)]) + .then((result) => { + return P.resolve({ + remaining: result.count + }) + }) + .catch((err) => { + if (err.errno === 1643) { + throw error.notFound() + } + + throw err + }) + } + return MySql } diff --git a/lib/db/patch.js b/lib/db/patch.js index 72c10be7..7bed5551 100644 --- a/lib/db/patch.js +++ b/lib/db/patch.js @@ -4,4 +4,4 @@ // The expected patch level of the database. Update if you add a new // patch in the ./schema/ directory. -module.exports.level = 74 +module.exports.level = 75 diff --git a/lib/db/schema/patch-074-075.sql b/lib/db/schema/patch-074-075.sql new file mode 100644 index 00000000..dcdeb38b --- /dev/null +++ b/lib/db/schema/patch-074-075.sql @@ -0,0 +1,74 @@ +SET NAMES utf8mb4 COLLATE utf8mb4_bin; + +CREATE TABLE IF NOT EXISTS recoveryCodes ( + uid BINARY(16) NOT NULL, + codeHash BINARY(64) NOT NULL, + createdAt BIGINT UNSIGNED NOT NULL, + UNIQUE KEY (`uid`, `codeHash`) +) ENGINE=InnoDB; + +CREATE PROCEDURE `deleteRecoveryCodes_1` ( + IN `uidArg` BINARY(16) +) +BEGIN + + DECLARE EXIT HANDLER FOR SQLEXCEPTION + BEGIN + ROLLBACK; + RESIGNAL; + END; + + START TRANSACTION; + + SET @accountCount = 0; + + -- Signal error if no user found + SELECT COUNT(*) INTO @accountCount FROM `accounts` WHERE `uid` = `uidArg`; + IF @accountCount = 0 THEN + SIGNAL SQLSTATE '45000' SET MYSQL_ERRNO = 1643, MESSAGE_TEXT = 'Can not generate recovery codes for unknown user.'; + END IF; + + -- Delete all current recovery codes + DELETE FROM `recoveryCodes` WHERE `uid` = `uidArg`; + + COMMIT; +END; + +CREATE PROCEDURE `createRecoveryCode_1` ( + IN `uidArg` BINARY(16), + IN `codeHashArg` BINARY(64) +) +BEGIN + + INSERT INTO recoveryCodes (uid, codeHash, createdAt) VALUES (uidArg, codeHashArg, NOW()); + +END; + +CREATE PROCEDURE `consumeRecoveryCode_1` ( + IN `uidArg` BINARY(16), + IN `codeHashArg` BINARY(64) +) +BEGIN + DECLARE EXIT HANDLER FOR SQLEXCEPTION + BEGIN + ROLLBACK; + RESIGNAL; + END; + + START TRANSACTION; + + SET @deletedCount = 0; + + DELETE FROM `recoveryCodes` WHERE `uid` = `uidArg` AND `codeHash` = `codeHashArg`; + + SELECT ROW_COUNT() INTO @deletedCount; + IF @deletedCount = 0 THEN + SIGNAL SQLSTATE '45000' SET MYSQL_ERRNO = 1643, MESSAGE_TEXT = 'Unknown recovery code.'; + END IF; + + SELECT COUNT(*) AS count FROM `recoveryCodes` WHERE `uid` = `uidArg`; + + COMMIT; +END; + +UPDATE dbMetadata SET value = '75' WHERE name = 'schema-patch-level'; \ No newline at end of file diff --git a/lib/db/schema/patch-075-074.sql b/lib/db/schema/patch-075-074.sql new file mode 100644 index 00000000..62188fdd --- /dev/null +++ b/lib/db/schema/patch-075-074.sql @@ -0,0 +1,9 @@ +-- SET NAMES utf8mb4 COLLATE utf8mb4_bin; + +-- DROP TABLE recoveryCodes; +-- DROP PROCEDURE deleteRecoveryCodes_1; +-- DROP PROCEDURE createRecoveryCode_1; +-- DROP PROCEDURE consumeRecoveryCode_1; + +-- UPDATE dbMetadata SET value = '74' WHERE name = 'schema-patch-level'; + diff --git a/lib/db/util.js b/lib/db/util.js index 8cd45b12..4b16bae4 100644 --- a/lib/db/util.js +++ b/lib/db/util.js @@ -5,6 +5,8 @@ 'use strict' const crypto = require('crypto') +const P = require('../promise') +const randomBytes = P.promisify(require('crypto').randomBytes) const BOUNCE_TYPES = new Map([ ['__fxa__unmapped', 0], // a bounce type we don't yet recognize @@ -34,7 +36,8 @@ const BOUNCE_SUB_TYPES = new Map([ const VERIFICATION_METHODS = new Map([ ['email', 0], // sign-in confirmation email link ['email-2fa', 1], // sign-in confirmation email code (token code) - ['totp-2fa', 2] // TOTP code + ['totp-2fa', 2], // TOTP code + ['recovery-code', 3] // Recovery code ]) // If you modify one of these maps, modify the other. @@ -86,5 +89,28 @@ module.exports = { hash.update(arg) }) return hash.digest() + }, + + createHashSha512 () { + const hash = crypto.createHash('sha512') + const args = [...arguments] + args.forEach((arg) => { + hash.update(arg) + }) + return hash.digest() + }, + + generateRecoveryCodes(count) { + const randomByteCodes = [] + for (let i = 0; i < count; i++) { + randomByteCodes.push(randomBytes(4)) + } + + return P.all(randomByteCodes) + .then((result) => { + return result.map((randomCode) => { + return randomCode.toString('hex') + }) + }) } }