diff --git a/bin/key_server.js b/bin/key_server.js index 9b9b6d1a..19400543 100644 --- a/bin/key_server.js +++ b/bin/key_server.js @@ -39,6 +39,7 @@ function main() { var error = require('../lib/error') var Token = require('../lib/tokens')(log, config) var Password = require('../lib/crypto/password')(log, config) + var UnblockCode = require('../lib/crypto/base36')(config.signinUnblock.codeLength) var signer = require('../lib/signer')(config.secretKeyFile, config.domain) var serverPublicKeys = { @@ -89,7 +90,8 @@ function main() { Token.KeyFetchToken, Token.AccountResetToken, Token.PasswordForgotToken, - Token.PasswordChangeToken + Token.PasswordChangeToken, + UnblockCode ) DB.connect(config[config.db.backend]) diff --git a/config/index.js b/config/index.js index a59e575b..3d2599b3 100644 --- a/config/index.js +++ b/config/index.js @@ -498,6 +498,57 @@ var conf = convict({ default: /.+@mozilla\.com$/, env: 'LASTACCESSTIME_UPDATES_EMAIL_ADDRESSES' } + }, + signinUnblock: { + codeLength: { + doc: 'Number of base36 digits to make up an unblockCode', + default: 8, + env: 'SIGNIN_UNBLOCK_CODE_LENGTH' + }, + codeLifetime: { + doc: 'How long an unblockCode should be valid for', + format: 'duration', + default: '1 hour', + env: 'SIGNIN_UNBLOCK_CODE_LIFETIME' + }, + enabled: { + default: true + }, + allowedEmailAddresses: { + doc: 'If feature enabled, allow sign-in unblock for email addresses matching this regex.', + format: RegExp, + default: '.+@mozilla\\.com$', + env: 'SIGNIN_UNBLOCK_ALLOWED_EMAILS' + }, + forcedEmailAddresses: { + doc: 'If feature enabled, force sign-in unblock for email addresses matching this regex.', + format: RegExp, + default: '^$', // default is no one + env: 'SIGNIN_UNBLOCK_FORCED_EMAILS' + }, + sampleRate: { + doc: 'signin unblock sample rate, between 0.0 and 1.0', + default: 1.0, + env: 'SIGNIN_UNBLOCK_RATE' + }, + supportedClients: { + doc: 'support sign-in unblock for only these clients', + format: Array, + default: [ + 'web', + 'oauth', + 'iframe', + 'fx_firstrun_v1', + 'fx_firstrun_v2', + 'fx_desktop_v1', + 'fx_desktop_v2', + 'fx_desktop_v3', + 'fx_ios_v1', + 'fx_ios_v2', + 'fx_fennec_v1' + ], + env: 'SIGNIN_UNBLOCK_SUPPORTED_CLIENTS' + } } }) @@ -518,6 +569,7 @@ conf.set('smtp.passwordResetUrl', conf.get('contentServer.url') + '/v1/complete_ conf.set('smtp.initiatePasswordResetUrl', conf.get('contentServer.url') + '/reset_password') conf.set('smtp.initiatePasswordChangeUrl', conf.get('contentServer.url') + '/settings/change_password') conf.set('smtp.verifyLoginUrl', conf.get('contentServer.url') + '/complete_signin') +conf.set('smtp.reportSignInUrl', conf.get('contentServer.url') + '/report_signin') conf.set('isProduction', conf.get('env') === 'prod') diff --git a/docs/api.md b/docs/api.md index f05844b4..46e4f5eb 100644 --- a/docs/api.md +++ b/docs/api.md @@ -86,6 +86,7 @@ The currently-defined error responses are: * status code 400, errno 124: session already registered by another device * status code 400, errno 125: request blocked for security reasons * status code 400, errno 126: account must be reset +* status code 400, errno 127: invalid unblock code * status code 503, errno 201: service temporarily unavailable to due high load (see [backoff protocol](#backoff-protocol)) * status code 503, errno 202: feature has been disabled for operational reasons * any status code, errno 999: unknown error @@ -318,6 +319,7 @@ ___Parameters___ * authPW - the PBKDF2/HKDF stretched password as a hex string * service - (optional) opaque alphanumeric token to be included in verification links * reason - (optional) alphanumeric string indicating the reason for establishing a new session; may be "login" (the default) or "reconnect" +* unblockCode - (optional) base36 code used to unblock certain rate-limitings ### Request @@ -363,6 +365,7 @@ Failing requests may be due to the following errors: * status code 413, errno 113: request body too large * status code 400, errno 120: incorrect email case * status code 400, errno 126: account must be reset +* status code 400, errno 127: invalid unblock code ## GET /v1/account/keys diff --git a/lib/crypto/base36.js b/lib/crypto/base36.js new file mode 100644 index 00000000..b10234a2 --- /dev/null +++ b/lib/crypto/base36.js @@ -0,0 +1,32 @@ +/* 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 crypto = require('crypto') + +function base36(len) { + let out = [] + while (out.length < len) { + let rand = crypto.randomBytes(len) + let randLen = rand.length + for (let i = 0; i < randLen; i++) { + let b = rand[i] + // 252-256 skews the base36 distribution, so skip those bytes + if (b < 252) { + out.push((b % 36).toString(36)) + if (out.length === len) { + break + } + } + } + } + return out.join('').toUpperCase() +} + +module.exports = (len) => { + return () => { + return base36(len) + } +} diff --git a/lib/customs.js b/lib/customs.js index 78fd9369..bf11ebfe 100644 --- a/lib/customs.js +++ b/lib/customs.js @@ -55,14 +55,16 @@ module.exports = function (log, error) { // log a flow event that user got blocked. log.flowEvent('customs.blocked', request) + var unblock = !!result.unblock if (result.retryAfter) { // create a localized retryAfterLocalized value from retryAfter, for example '713' becomes '12 minutes'. var retryAfterLocalized = localizeTimestamp.format(Date.now() + (result.retryAfter * 1000), request.headers['accept-language']) - throw error.tooManyRequests(result.retryAfter, retryAfterLocalized) + throw error.tooManyRequests(result.retryAfter, retryAfterLocalized, unblock) + } else { + throw error.requestBlocked(unblock) } - throw error.requestBlocked() } if (result.suspect) { request.app.isSuspiciousRequest = true diff --git a/lib/db.js b/lib/db.js index eaf67ff0..5305cfd5 100644 --- a/lib/db.js +++ b/lib/db.js @@ -19,7 +19,8 @@ module.exports = function ( KeyFetchToken, AccountResetToken, PasswordForgotToken, - PasswordChangeToken) { + PasswordChangeToken, + UnblockCode) { const features = require('./features')(config) @@ -803,6 +804,49 @@ module.exports = function ( return this.pool.get('/securityEvents/' + params.uid.toString('hex') + '/ip/' + params.ipAddr) } + DB.prototype.createUnblockCode = function (uid) { + log.trace({ + op: 'DB.createUnblockCode', + uid: uid + }) + var unblock = UnblockCode() + return this.pool.put('/account/' + uid.toString('hex') + '/unblock/' + unblock) + .then( + () => { + return unblock + }, + (err) => { + // duplicates should be super rare, but it's feasible that a + // uid already has an existing unblockCode. Just try again. + if (isRecordAlreadyExistsError(err)) { + log.error({ + op: 'DB.createUnblockCode.duplicate', + err: err, + uid: uid + }) + return this.createUnblockCode(uid) + } + throw err + } + ) + } + + DB.prototype.consumeUnblockCode = function (uid, code) { + log.trace({ + op: 'DB.consumeUnblockCode', + uid: uid + }) + return this.pool.del('/account/' + uid.toString('hex') + '/unblock/' + code) + .catch( + function (err) { + if (isNotFoundError(err)) { + throw error.invalidUnblockCode() + } + throw err + } + ) + } + return DB } diff --git a/lib/error.js b/lib/error.js index c3114113..033e7bf3 100644 --- a/lib/error.js +++ b/lib/error.js @@ -24,6 +24,7 @@ var ERRNO = { INVALID_REQUEST_SIGNATURE: 109, INVALID_TIMESTAMP: 111, INVALID_TOKEN: 110, + INVALID_UNBLOCK_CODE: 127, INVALID_VERIFICATION_CODE: 105, MISSING_CONTENT_LENGTH_HEADER: 112, MISSING_PARAMETER: 108, @@ -327,7 +328,7 @@ AppError.requestBodyTooLarge = function () { }) } -AppError.tooManyRequests = function (retryAfter, retryAfterLocalized) { +AppError.tooManyRequests = function (retryAfter, retryAfterLocalized, canUnblock) { if (!retryAfter) { retryAfter = 30 } @@ -340,6 +341,11 @@ AppError.tooManyRequests = function (retryAfter, retryAfterLocalized) { extraData.retryAfterLocalized = retryAfterLocalized } + if (canUnblock) { + extraData.verificationMethod = 'email-captcha' + extraData.verificationReason = 'login' + } + return new AppError( { code: 429, @@ -354,13 +360,20 @@ AppError.tooManyRequests = function (retryAfter, retryAfterLocalized) { ) } -AppError.requestBlocked = function () { +AppError.requestBlocked = function (canUnblock) { + var extra + if (canUnblock) { + extra = { + verificationMethod: 'email-captcha', + verificationReason: 'login' + } + } return new AppError({ code: 400, error: 'Request blocked', errno: ERRNO.REQUEST_BLOCKED, message: 'The request was blocked for security reasons' - }) + }, extra) } AppError.serviceUnavailable = function (retryAfter) { @@ -448,5 +461,14 @@ AppError.deviceSessionConflict = function () { ) } +AppError.invalidUnblockCode = function () { + return new AppError({ + code: 400, + error: 'Bad Request', + errno: ERRNO.INVALID_UNBLOCK_CODE, + message: 'Invalid unblock code' + }) +} + module.exports = AppError module.exports.ERRNO = ERRNO diff --git a/lib/features.js b/lib/features.js index 34d13e26..6fec5816 100644 --- a/lib/features.js +++ b/lib/features.js @@ -9,6 +9,7 @@ const crypto = require('crypto') module.exports = config => { const lastAccessTimeUpdates = config.lastAccessTimeUpdates const signinConfirmation = config.signinConfirmation + const signinUnblock = config.signinUnblock return { /** @@ -52,7 +53,6 @@ module.exports = config => { // edge-cases in device login flows that haven't been fully tested. // Temporarily avoid them for regular users by checking the `context` flag, // and create pre-verified sessions for unsupported clients. - // This check will go away in the final version of this feature. const context = request.payload && request.payload.metricsContext && request.payload.metricsContext.context @@ -64,6 +64,43 @@ module.exports = config => { return isSampledUser(signinConfirmation.sample_rate, uid, 'signinConfirmation') }, + + /** + * Returns whether or not to use signin unblock feature on a request. + * + * @param account + * @param config + * @param request + * @returns {boolean} + */ + isSigninUnblockEnabledForUser(uid, email, request) { + if (! signinUnblock.enabled) { + return false + } + + if (signinUnblock.forcedEmailAddresses && signinUnblock.forcedEmailAddresses.test(email)) { + return true + } + + if (signinUnblock.allowedEmailAddresses.test(email)) { + return true + } + + // While we're testing this feature, there may be some funky + // edge-cases in device login flows that haven't been fully tested. + // Temporarily avoid them for regular users by checking the `context` flag, + const context = request.payload && + request.payload.metricsContext && + request.payload.metricsContext.context + + if (signinUnblock.supportedClients.indexOf(context) === -1) { + return false + } + + // Check to see if user in roll-out cohort. + return isSampledUser(signinUnblock.sampleRate, uid, 'signinUnblock') + }, + /** * Predicate that indicates whether a user belongs to the sampled cohort, * based on a sample rate, their uid and a key that identifies the feature. diff --git a/lib/log.js b/lib/log.js index dcaf4ac9..c30f99da 100644 --- a/lib/log.js +++ b/lib/log.js @@ -25,7 +25,10 @@ var ACTIVITY_FLOW_EVENTS = Object.keys(ALWAYS_ACTIVITY_FLOW_EVENTS) }, { // These activity events are flow events when there is a flowId 'account.keyfetch': true, - 'account.signed': true + 'account.login.sentUnblockCode': true, + 'account.login.confirmedUnblockCode': true, + 'account.signed': true, + 'device.created': true }) function unbuffer(object) { diff --git a/lib/mailer.js b/lib/mailer.js index b4d4fa75..6681885d 100644 --- a/lib/mailer.js +++ b/lib/mailer.js @@ -103,6 +103,23 @@ module.exports = function (config, log) { } )) } + mailer.sendUnblockCode = function (account, unblockCode, opts) { + return P.resolve(mailer.unblockCodeEmail( + { + acceptLanguage: opts.acceptLanguage || defaultLanguage, + email: account.email, + ip: opts.ip, + location: opts.location, + timeZone: opts.timeZone, + uaBrowser: opts.uaBrowser, + uaBrowserVersion: opts.uaBrowserVersion, + uaOS: opts.uaOS, + uaOSVersion: opts.uaOSVersion, + uid: account.uid.toString('hex'), + unblockCode: unblockCode + } + )) + } return mailer } ) diff --git a/lib/routes/account.js b/lib/routes/account.js index 3bbc298e..53b7daa9 100644 --- a/lib/routes/account.js +++ b/lib/routes/account.js @@ -2,11 +2,14 @@ * 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' + var validators = require('./validators') var HEX_STRING = validators.HEX_STRING var BASE64_JWT = validators.BASE64_JWT var DISPLAY_SAFE_UNICODE = validators.DISPLAY_SAFE_UNICODE var URLSAFEBASE64 = validators.URLSAFEBASE64 +var BASE_36 = validators.BASE_36 var PUSH_PAYLOADS_SCHEMA_PATH = '../../docs/pushpayloads.schema.json' // An arbitrary, but very generous, limit on the number of active sessions. @@ -56,7 +59,9 @@ module.exports = function ( }) const features = require('../features')(config) - var securityHistoryEnabled = config.securityHistory && config.securityHistory.enabled + const securityHistoryEnabled = config.securityHistory && config.securityHistory.enabled + const unblockCodeLifetime = config.signinUnblock && config.signinUnblock.codeLifetime || 0 + const unblockCodeLen = config.signinUnblock && config.signinUnblock.codeLength || 0 var routes = [ { @@ -354,6 +359,7 @@ module.exports = function ( redirectTo: isA.string().uri().optional(), resume: isA.string().optional(), reason: isA.string().max(16).optional(), + unblockCode: isA.string().regex(BASE_36).length(unblockCodeLen).optional(), metricsContext: metricsContext.schema } }, @@ -380,7 +386,7 @@ module.exports = function ( var redirectTo = request.payload.redirectTo var resume = request.payload.resume var tokenVerificationId = crypto.randomBytes(16) - var emailRecord, sessions, sessionToken, keyFetchToken, mustVerifySession, doSigninConfirmation, emailSent + var emailRecord, sessions, sessionToken, keyFetchToken, mustVerifySession, doSigninConfirmation, emailSent, unblockCode, customsErr, allowSigninUnblock, didSigninUnblock var ip = request.app.clientAddress metricsContext.validate(request) @@ -393,8 +399,11 @@ module.exports = function ( }) } - customs.check(request, email, 'accountLogin') + checkIsBlockForced() + .then(() => customs.check(request, email, 'accountLogin')) + .catch(checkUnblockCode) .then(readEmailRecord) + .then(checkEmailAndPassword) .then(checkSecurityHistory) .then(checkNumberOfActiveSessions) .then(createSessionToken) @@ -405,52 +414,72 @@ module.exports = function ( .then(sendVerifyLoginEmail) .then(recordSecurityEvent) .then(createResponse) + .catch(gateSigninUnblock) .done(reply, reply) + function checkIsBlockForced () { + let forced = config.signinUnblock && config.signinUnblock.enabled && config.signinUnblock.forcedEmailAddresses + + if (forced && forced.test(email)) { + return P.reject(error.requestBlocked(true)) + } + + return P.resolve() + } + + function checkUnblockCode (e) { + var method = e.output.payload.verificationMethod + if (method === 'email-captcha') { + // only set `unblockCode` if it is required from customs + unblockCode = request.payload.unblockCode + if (unblockCode) { + unblockCode = unblockCode.toUpperCase() + } + customsErr = e + return + } + throw e + } + function readEmailRecord () { return db.emailRecord(email) .then( function (result) { emailRecord = result - // Session token verification is only enabled for certain users during phased rollout. - // Even when it is enabled, we only do the email challenge if: - // * the request wants keys, since unverified sessions are fine to use for e.g. oauth login. - // * the email is verified, since content-server triggers a resend of the verification - // email on unverified accounts, which doubles as sign-in confirmation. - if (! features.isSigninConfirmationEnabledForUser(emailRecord.uid, emailRecord.email, request)) { - tokenVerificationId = undefined - mustVerifySession = false - doSigninConfirmation = false - } else { - // The user doesn't *have* to verify their session if they're not requesting keys, - // but we still create it with a non-null tokenVerificationId, so it will still - // be considered unverified. This prevents the session from being used for sync - // unless the user explicitly requests us to resend the confirmation email, and completes it. - mustVerifySession = requestHelper.wantsKeys(request) - doSigninConfirmation = mustVerifySession && emailRecord.emailVerified - } - - if(email !== emailRecord.email) { - customs.flag(request.app.clientAddress, { - email: email, - errno: error.ERRNO.INCORRECT_PASSWORD - }) - throw error.incorrectPassword(emailRecord.email, email) - } - - return checkPassword(emailRecord, authPW, request.app.clientAddress) - .then( - function (match) { - if (! match) { - throw error.incorrectPassword(emailRecord.email, email) + allowSigninUnblock = features.isSigninUnblockEnabledForUser(emailRecord.uid, email, request) + if (allowSigninUnblock && unblockCode) { + return db.consumeUnblockCode(emailRecord.uid, unblockCode) + .then( + (code) => { + if (Date.now() - code.createdAt > unblockCodeLifetime) { + log.info({ + op: 'Account.login.unblockCode.expired', + uid: emailRecord.uid.toString('hex') + }) + throw error.invalidUnblockCode() + } + didSigninUnblock = true + return log.activityEvent('account.login.confirmedUnblockCode', request, { + uid: emailRecord.uid.toString('hex') + }) } - - return log.activityEvent('account.login', request, { - uid: emailRecord.uid.toString('hex') - }) - } - ) + ) + .catch( + (err) => { + if (err.errno === error.ERRNO.UNBLOCK_CODE_INVALID) { + customs.flag(request.app.clientAddress, { + email: email, + errno: err.errno + }) + } + throw err + } + ) + } + if (!didSigninUnblock && customsErr) { + throw customsErr + } }, function (err) { if (err.errno === error.ERRNO.ACCOUNT_UNKNOWN) { @@ -530,6 +559,51 @@ module.exports = function ( ) } + function checkEmailAndPassword () { + // Session token verification is only enabled for certain users during phased rollout. + // + // If the user went through the sigin-unblock flow, they have already verified their email. + // No need to also require confirmation afterwards. + // + // Even when it is enabled, we only do the email challenge if: + // * the request wants keys, since unverified sessions are fine to use for e.g. oauth login. + // * the email is verified, since content-server triggers a resend of the verification + // email on unverified accounts, which doubles as sign-in confirmation. + if (didSigninUnblock || !features.isSigninConfirmationEnabledForUser(emailRecord.uid, emailRecord.email, request)) { + tokenVerificationId = undefined + mustVerifySession = false + doSigninConfirmation = false + } else { + // The user doesn't *have* to verify their session if they're not requesting keys, + // but we still create it with a non-null tokenVerificationId, so it will still + // be considered unverified. This prevents the session from being used for sync + // unless the user explicitly requests us to resend the confirmation email, and completes it. + mustVerifySession = requestHelper.wantsKeys(request) + doSigninConfirmation = mustVerifySession && emailRecord.emailVerified + } + + if(email !== emailRecord.email) { + customs.flag(request.app.clientAddress, { + email: email, + errno: error.ERRNO.INCORRECT_PASSWORD + }) + throw error.incorrectPassword(emailRecord.email, email) + } + + return checkPassword(emailRecord, authPW, request.app.clientAddress) + .then( + function (match) { + if (! match) { + throw error.incorrectPassword(emailRecord.email, email) + } + + return log.activityEvent('account.login', request, { + uid: emailRecord.uid.toString('hex') + }) + } + ) + } + function checkNumberOfActiveSessions () { return db.sessions(emailRecord.uid) .then( @@ -741,6 +815,18 @@ module.exports = function ( } return P.resolve(response) } + + function gateSigninUnblock (err) { + // customs.check will always add these properties if the + // customs server has not rate-limited unblock. Nonetheless, + // we shouldn't signal to the content-server that it is + // possible to unblock the user if the feature is not allowed. + if (!allowSigninUnblock && err.output && err.output.payload) { + delete err.output.payload.verificationMethod + delete err.output.payload.verificationReason + } + throw err + } } }, { @@ -1537,6 +1623,94 @@ module.exports = function ( reply(error.gone()) } }, + { + method: 'POST', + path: '/account/login/send_unblock_code', + config: { + validate: { + payload: { + email: validators.email().required(), + metricsContext: metricsContext.schema + } + } + }, + handler: function (request, reply) { + log.begin('Account.SendUnblockCode', request) + + var email = request.payload.email + var ip = request.app.clientAddress + var emailRecord + + return customs.check(request, email, 'sendUnblockCode') + .then(lookupAccount) + .then(createUnblockCode) + .then(mailUnblockCode) + .then(() => log.activityEvent('account.login.sentUnblockCode', request, { + uid: emailRecord.uid.toString('hex') + })) + .done(() => { + reply({}) + }, reply) + + function lookupAccount() { + return db.emailRecord(email) + .then((record) => { + emailRecord = record + return record.uid + }) + } + + function createUnblockCode(uid) { + + if (features.isSigninUnblockEnabledForUser(uid, email, request)) { + return db.createUnblockCode(uid) + } else { + throw error.featureNotEnabled() + } + } + + function mailUnblockCode(code) { + return getGeoData(ip) + .then((geoData) => { + return mailer.sendUnblockCode(emailRecord, code, userAgent.call({ + acceptLanguage: request.app.acceptLanguage, + ip: ip, + location: geoData.location, + timeZone: geoData.timeZone + }, request.headers['user-agent'], log)) + }) + } + } + }, + { + method: 'POST', + path: '/account/login/reject_unblock_code', + config: { + validate: { + payload: { + uid: isA.string().max(32).regex(HEX_STRING).required(), + unblockCode: isA.string().regex(BASE_36).length(unblockCodeLen).required() + } + } + }, + handler: function (request, reply) { + var uid = Buffer(request.payload.uid, 'hex') + var code = request.payload.unblockCode.toUpperCase() + + log.begin('Account.RejectUnblockCode', request) + db.consumeUnblockCode(uid, code) + .then( + () => { + log.info({ + op: 'account.login.rejectedUnblockCode', + uid: request.payload.uid, + unblockCode: code + }) + return {} + } + ).done(reply, reply) + } + }, { method: 'POST', path: '/account/reset', diff --git a/lib/routes/validators.js b/lib/routes/validators.js index fc6d1b3c..5e9f5db9 100644 --- a/lib/routes/validators.js +++ b/lib/routes/validators.js @@ -14,6 +14,8 @@ module.exports.BASE64_JWT = /^(?:[a-zA-Z0-9-_]+[=]{0,2}\.){2}[a-zA-Z0-9-_]+[=]{0 module.exports.URLSAFEBASE64 = /^[a-zA-Z0-9-_]*$/ +module.exports.BASE_36 = /^[a-zA-Z0-9]*$/ + // Match display-safe unicode characters. // We're pretty liberal with what's allowed in a unicode string, // but we exclude the following classes of characters: diff --git a/test/local/account_routes.js b/test/local/account_routes.js index 94241ef1..157b1bec 100644 --- a/test/local/account_routes.js +++ b/test/local/account_routes.js @@ -2,6 +2,8 @@ * 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' + var sinon = require('sinon') var test = require('tap').test @@ -36,6 +38,7 @@ var makeRoutes = function (options, requireMocks) { } config.lastAccessTimeUpdates = {} config.signinConfirmation = config.signinConfirmation || {} + config.signinUnblock = config.signinUnblock || {} var log = options.log || mocks.mockLog() var Password = options.Password || require('../../lib/crypto/password')(log, config) @@ -698,13 +701,18 @@ test('/account/create', function (t) { }) test('/account/login', function (t) { - t.plan(5) + t.plan(6) var config = { newLoginNotificationEnabled: true, securityHistory: { enabled: true }, - signinConfirmation: {} + signinConfirmation: {}, + signinUnblock: { + allowedEmailAddresses: /^.*$/, + codeLifetime: 1000, + enabled: true + } } var mockRequest = mocks.mockRequest({ query: { @@ -738,6 +746,21 @@ test('/account/login', function (t) { } } }) + var mockRequestWithUnblockCode = mocks.mockRequest({ + query: {}, + payload: { + authPW: crypto.randomBytes(32).toString('hex'), + email: 'test@mozilla.com', + unblockCode: 'ABCD1234', + service: 'dcdb5ae7add825d2', + reason: 'signin', + metricsContext: { + flowBeginTime: Date.now(), + flowId: 'F1031DF1031DF1031DF1031DF1031DF1031DF1031DF1031DF1031DF1031DF103', + service: 'dcdb5ae7add825d2' + } + } + }) var keyFetchTokenId = crypto.randomBytes(16) var sessionTokenId = crypto.randomBytes(16) var uid = uuid.v4('binary') @@ -770,16 +793,15 @@ test('/account/login', function (t) { }) var mockMailer = mocks.mockMailer() var mockPush = mocks.mockPush() + var mockCustoms = { + check: () => P.resolve() + } var accountRoutes = makeRoutes({ checkPassword: function () { return P.resolve(true) }, config: config, - customs: { - check: function () { - return P.resolve() - } - }, + customs: mockCustoms, db: mockDB, log: mockLog, mailer: mockMailer, @@ -1400,7 +1422,6 @@ test('/account/login', function (t) { t.equal(record, undefined, 'log.info was not called for Account.history.verified') }) }) - }) t.test('records security event', function (t) { @@ -1417,6 +1438,93 @@ test('/account/login', function (t) { t.equal(securityQuery.name, 'account.login') }) }) + + t.test('blocked by customs', (t) => { + t.plan(2) + t.test('can unblock', (t) => { + t.plan(2) + mockCustoms.check = () => P.reject(error.requestBlocked(true)) + t.test('signin unblock disabled', (t) => { + t.plan(4) + config.signinUnblock.enabled = false + return runTest(route, mockRequest, (err) => { + t.equal(err.errno, error.ERRNO.REQUEST_BLOCKED, 'correct errno is returned') + t.equal(err.output.statusCode, 400, 'correct status code is returned') + t.equal(err.output.payload.verificationMethod, undefined, 'no verificationMethod') + t.equal(err.output.payload.verificationReason, undefined, 'no verificationReason') + }) + }) + + t.test('signin unblock enabled', (t) => { + t.plan(2) + config.signinUnblock.enabled = true + + t.test('without unblock code', (t) => { + t.plan(4) + return runTest(route, mockRequest, (err) => { + t.equal(err.errno, error.ERRNO.REQUEST_BLOCKED, 'correct errno is returned') + t.equal(err.output.statusCode, 400, 'correct status code is returned') + t.equal(err.output.payload.verificationMethod, 'email-captcha', 'with verificationMethod') + t.equal(err.output.payload.verificationReason, 'login', 'with verificationReason') + }) + }) + + t.test('with unblock code', (t) => { + t.plan(3) + t.test('invalid code', (t) => { + t.plan(2) + mockDB.consumeUnblockCode = () => P.reject(error.invalidUnblockCode()) + return runTest(route, mockRequestWithUnblockCode, (err) => { + t.equal(err.errno, error.ERRNO.INVALID_UNBLOCK_CODE, 'correct errno is returned') + t.equal(err.output.statusCode, 400, 'correct status code is returned') + }) + }) + + t.test('expired code', (t) => { + mockDB.consumeUnblockCode = () => P.resolve({ createdAt: Date.now() - config.signinUnblock.codeLifetime - 1 }) + return runTest(route, mockRequestWithUnblockCode, (err) => { + t.equal(err.errno, error.ERRNO.INVALID_UNBLOCK_CODE, 'correct errno is returned') + t.equal(err.output.statusCode, 400, 'correct status code is returned') + }) + }) + + t.test('valid code', (t) => { + t.plan(1) + mockDB.consumeUnblockCode = () => P.resolve({ createdAt: Date.now() }) + return runTest(route, mockRequestWithUnblockCode, (res) => { + t.ok(!(res instanceof Error), 'successful login') + }) + }) + }) + }) + }) + + t.test('cannot unblock', (t) => { + t.plan(2) + mockCustoms.check = () => P.reject(error.requestBlocked(false)) + config.signinUnblock.enabled = true + + t.test('without an unblock code', (t) => { + t.plan(4) + return runTest(route, mockRequest, (err) => { + t.equal(err.errno, error.ERRNO.REQUEST_BLOCKED, 'correct errno is returned') + t.equal(err.output.statusCode, 400, 'correct status code is returned') + t.equal(err.output.payload.verificationMethod, undefined, 'no verificationMethod') + t.equal(err.output.payload.verificationReason, undefined, 'no verificationReason') + }) + }) + + t.test('with unblock code', (t) => { + t.plan(4) + return runTest(route, mockRequestWithUnblockCode, (err) => { + t.equal(err.errno, error.ERRNO.REQUEST_BLOCKED, 'correct errno is returned') + t.equal(err.output.statusCode, 400, 'correct status code is returned') + t.equal(err.output.payload.verificationMethod, undefined, 'no verificationMethod') + t.equal(err.output.payload.verificationReason, undefined, 'no verificationReason') + }) + }) + }) + }) }) test('/recovery_email/verify_code', function (t) { @@ -1759,3 +1867,95 @@ test('/account/devices', function (t) { }) }) +test('/account/login/send_unblock_code', function (t) { + t.plan(2) + var uid = uuid.v4('binary').toString('hex') + var mockRequest = mocks.mockRequest({ + payload: { + email: TEST_EMAIL, + metricsContext: { + context: 'fx_desktop_v3', + flowBeginTime: Date.now(), + flowId: 'F1031DF1031DF1031DF1031DF1031DF1031DF1031DF1031DF1031DF1031DF103', + entrypoint: 'preferences', + utmContent: 'some-content-string' + } + } + }) + var mockMailer = mocks.mockMailer() + var mockDb = mocks.mockDB({ + uid: uid, + email: TEST_EMAIL + }) + var config = { + signinUnblock: { + allowedEmailAddresses: /^.*$/ + } + } + var accountRoutes = makeRoutes({ + config: config, + db: mockDb, + mailer: mockMailer + }) + var route = getRoute(accountRoutes, '/account/login/send_unblock_code') + + t.test('signin unblock enabled', function (t) { + t.plan(9) + config.signinUnblock.enabled = true + return runTest(route, mockRequest, function (response) { + t.ok(!(response instanceof Error), response.stack) + t.deepEqual(response, {}, 'response has no keys') + + t.equal(mockDb.emailRecord.callCount, 1, 'db.emailRecord called') + t.equal(mockDb.emailRecord.args[0][0], TEST_EMAIL) + + t.equal(mockDb.createUnblockCode.callCount, 1, 'db.createUnblockCode called') + var dbArgs = mockDb.createUnblockCode.args[0] + t.equal(dbArgs.length, 1) + t.equal(dbArgs[0], uid) + + t.equal(mockMailer.sendUnblockCode.callCount, 1, 'called mailer.sendUnblockCode') + var args = mockMailer.sendUnblockCode.args[0] + t.equal(args.length, 3, 'mailer.sendUnblockCode called with 3 args') + }) + }) + + t.test('signin unblock disabled', function (t) { + t.plan(2) + config.signinUnblock.enabled = false + + return runTest(route, mockRequest, function (err) { + t.equal(err.output.statusCode, 503, 'correct status code is returned') + t.equal(err.errno, error.ERRNO.FEATURE_NOT_ENABLED, 'correct errno is returned') + }) + }) +}) + +test('/account/login/reject_unblock_code', function (t) { + t.plan(6) + var uid = uuid.v4('binary').toString('hex') + var unblockCode = 'A1B2C3D4' + var mockRequest = mocks.mockRequest({ + payload: { + uid: uid, + unblockCode: unblockCode + } + }) + var mockDb = mocks.mockDB() + var accountRoutes = makeRoutes({ + db: mockDb + }) + var route = getRoute(accountRoutes, '/account/login/reject_unblock_code') + + return runTest(route, mockRequest, function (response) { + t.ok(!(response instanceof Error), response.stack) + t.deepEqual(response, {}, 'response has no keys') + + t.equal(mockDb.consumeUnblockCode.callCount, 1, 'consumeUnblockCode is called') + var args = mockDb.consumeUnblockCode.args[0] + t.equal(args.length, 2) + t.equal(args[0].toString('hex'), uid) + t.equal(args[1], unblockCode) + }) + +}) diff --git a/test/local/features_tests.js b/test/local/features_tests.js index 32700273..4982bd12 100644 --- a/test/local/features_tests.js +++ b/test/local/features_tests.js @@ -19,7 +19,8 @@ const crypto = { const config = { lastAccessTimeUpdates: {}, - signinConfirmation: {} + signinConfirmation: {}, + signinUnblock: {} } const features = proxyquire('../../lib/features', { @@ -30,10 +31,11 @@ test( 'interface is correct', t => { t.equal(typeof features, 'object', 'object type should be exported') - t.equal(Object.keys(features).length, 3, 'object should have two properties') + t.equal(Object.keys(features).length, 4, 'object should have four properties') t.equal(typeof features.isSampledUser, 'function', 'isSampledUser should be function') t.equal(typeof features.isLastAccessTimeEnabledForUser, 'function', 'isLastAccessTimeEnabledForUser should be function') t.equal(typeof features.isSigninConfirmationEnabledForUser, 'function', 'isSigninConfirmationEnabledForUser should be function') + t.equal(typeof features.isSigninUnblockEnabledForUser, 'function', 'isSigninUnblockEnabledForUser should be function') t.equal(crypto.createHash.callCount, 1, 'crypto.createHash should have been called once on require') let args = crypto.createHash.args[0] @@ -230,3 +232,50 @@ test( } ) +test( + 'isSigninUnblockEnabledForUser', + t => { + const uid = 'wibble' + const email = 'blee@mozilla.com' + const request = { + payload: { + metricsContext: { + context: 'iframe' + } + } + } + // First 27 characters are ignored, last 13 are 0.02 * 0xfffffffffffff + hashResult = '000000000000000000000000000051eb851eb852' + + const unblock = config.signinUnblock + + unblock.enabled = true + unblock.sampleRate = 0.02 + unblock.allowedEmailAddresses = /.+@notmozilla.com$/ + unblock.supportedClients = [ 'wibble', 'iframe' ] + t.equal(features.isSigninUnblockEnabledForUser(uid, email, request), false, 'should return false when email is not allowed and uid is not sampled') + + unblock.forcedEmailAddresses = /.+/ + t.equal(features.isSigninUnblockEnabledForUser(uid, email, request), true, 'should return true when forced on') + unblock.forcedEmailAddresses = /^$/ + + unblock.allowedEmailAddresses = /.+@mozilla.com$/ + t.equal(features.isSigninUnblockEnabledForUser(uid, email, request), true, 'should return true when email is allowed') + + unblock.allowedEmailAddresses = /.+@notmozilla.com$/ + unblock.sampleRate = 0.03 + t.equal(features.isSigninUnblockEnabledForUser(uid, email, request), true, 'should return when uid is sampled') + + + request.payload.metricsContext.context = '' + t.equal(features.isSigninUnblockEnabledForUser(uid, email, request), false, 'should return false when context is not supported') + + + request.payload.metricsContext.context = 'iframe' + unblock.enabled = false + t.equal(features.isSigninUnblockEnabledForUser(uid, email, request), false, 'should return false when feature is disabled') + + t.end() + } +) + diff --git a/test/mail_helper.js b/test/mail_helper.js index e7fc36c4..222bc8ae 100644 --- a/test/mail_helper.js +++ b/test/mail_helper.js @@ -24,6 +24,8 @@ require('simplesmtp').createSimpleServer( function (mail) { var link = mail.headers['x-link'] var rc = mail.headers['x-recovery-code'] + var rul = mail.headers['x-report-signin-link'] + var uc = mail.headers['x-unblock-code'] var vc = mail.headers['x-verify-code'] var name = emailName(mail.headers.to) if (vc) { @@ -32,6 +34,11 @@ require('simplesmtp').createSimpleServer( else if (rc) { console.log('\x1B[34m', link, '\x1B[39m') } + else if (uc) { + console.log('\x1B[36mUnblock code:', uc, '\x1B[39m') + console.log('\x1B[36mReport link:', rul, '\x1B[39m') + + } else { console.error('\x1B[31mNo verify code match\x1B[39m') console.error(mail) diff --git a/test/mocks.js b/test/mocks.js index 1314fc18..acbda666 100644 --- a/test/mocks.js +++ b/test/mocks.js @@ -6,33 +6,85 @@ * Shared helpers for mocking things out in the tests. */ -var sinon = require('sinon') -var extend = require('util')._extend -var P = require('../lib/promise') -var crypto = require('crypto') +const sinon = require('sinon') +const extend = require('util')._extend +const P = require('../lib/promise') +const crypto = require('crypto') -var CUSTOMS_METHOD_NAMES = ['check', 'checkAuthenticated', 'flag', 'reset'] +const CUSTOMS_METHOD_NAMES = [ + 'check', + 'checkAuthenticated', + 'flag', + 'reset' +] -var DB_METHOD_NAMES = ['account', 'createAccount', 'createDevice', 'createKeyFetchToken', - 'createPasswordForgotToken', 'createSessionToken', 'deleteAccount', - 'deleteDevice', 'deleteKeyFetchToken', 'deletePasswordChangeToken', - 'deleteVerificationReminder', 'devices', 'emailRecord', 'forgotPasswordVerified', - 'resetAccount', 'securityEvent', 'securityEvents', 'sessions', - 'sessionTokenWithVerificationStatus', 'updateDevice', 'updateLocale', - 'updateSessionToken', 'verifyEmail', 'verifyTokens',] +const DB_METHOD_NAMES = [ + 'account', + 'consumeUnblockCode', + 'createAccount', + 'createDevice', + 'createKeyFetchToken', + 'createPasswordForgotToken', + 'createSessionToken', + 'createUnblockCode', + 'deleteAccount', + 'deleteDevice', + 'deleteKeyFetchToken', + 'deletePasswordChangeToken', + 'deleteVerificationReminder', + 'devices', + 'emailRecord', + 'forgotPasswordVerified', + 'resetAccount', + 'securityEvent', + 'securityEvents', + 'sessions', + 'sessionTokenWithVerificationStatus', + 'updateDevice', + 'updateLocale', + 'updateSessionToken', + 'verifyEmail', + 'verifyTokens' +] -var LOG_METHOD_NAMES = ['trace', 'increment', 'info', 'error', 'begin', 'warn', 'timing', - 'activityEvent', 'flowEvent', 'notifyAttachedServices'] +const LOG_METHOD_NAMES = [ + 'activityEvent', + 'begin', + 'error', + 'flowEvent', + 'increment', + 'info', + 'notifyAttachedServices', + 'warn', + 'timing', + 'trace' +] -var MAILER_METHOD_NAMES = ['sendVerifyCode', 'sendVerifyLoginEmail', - 'sendNewDeviceLoginNotification', 'sendPasswordChangedNotification', - 'sendPasswordResetNotification', 'sendPostVerifyEmail'] +const MAILER_METHOD_NAMES = [ + 'sendNewDeviceLoginNotification', + 'sendPasswordChangedNotification', + 'sendPasswordResetNotification', + 'sendPostVerifyEmail', + 'sendUnblockCode', + 'sendVerifyCode', + 'sendVerifyLoginEmail' +] -var METRICS_CONTEXT_METHOD_NAMES = ['stash', 'gather', 'validate'] +const METRICS_CONTEXT_METHOD_NAMES = [ + 'gather', + 'stash', + 'validate' +] -var PUSH_METHOD_NAMES = ['notifyDeviceConnected', 'notifyDeviceDisconnected', 'notifyUpdate', - 'pushToAllDevices', 'pushToDevices', 'notifyPasswordChanged', - 'notifyPasswordReset'] +const PUSH_METHOD_NAMES = [ + 'notifyDeviceConnected', + 'notifyDeviceDisconnected', + 'notifyPasswordChanged', + 'notifyPasswordReset', + 'notifyUpdate', + 'pushToAllDevices', + 'pushToDevices' +] module.exports = { mockCustoms: mockObject(CUSTOMS_METHOD_NAMES), @@ -51,7 +103,7 @@ function mockDB (data, errors) { errors = errors || {} return mockObject(DB_METHOD_NAMES)({ - account: sinon.spy(function () { + account: sinon.spy(() => { return P.resolve({ email: data.email, emailCode: data.emailCode, @@ -61,7 +113,7 @@ function mockDB (data, errors) { wrapWrapKb: data.wrapWrapKb }) }), - createAccount: sinon.spy(function () { + createAccount: sinon.spy(() => { return P.resolve({ uid: data.uid, email: data.email, @@ -70,8 +122,8 @@ function mockDB (data, errors) { wrapWrapKb: data.wrapWrapKb }) }), - createDevice: sinon.spy(function () { - return P.resolve(Object.keys(data.device).reduce(function (result, key) { + createDevice: sinon.spy(() => { + return P.resolve(Object.keys(data.device).reduce((result, key) => { result[key] = data.device[key] return result }, { @@ -79,14 +131,14 @@ function mockDB (data, errors) { createdAt: data.deviceCreatedAt })) }), - createKeyFetchToken: sinon.spy(function () { + createKeyFetchToken: sinon.spy(() => { return P.resolve({ data: crypto.randomBytes(32), tokenId: data.keyFetchTokenId, uid: data.uid }) }), - createPasswordForgotToken: sinon.spy(function () { + createPasswordForgotToken: sinon.spy(() => { return P.resolve({ data: crypto.randomBytes(32), passCode: data.passCode, @@ -94,12 +146,12 @@ function mockDB (data, errors) { uid: data.uid }) }), - createSessionToken: sinon.spy(function () { + createSessionToken: sinon.spy(() => { return P.resolve({ data: crypto.randomBytes(32), email: data.email, emailVerified: data.emailVerified, - lastAuthAt: function () { + lastAuthAt: () => { return Date.now() }, tokenId: data.sessionTokenId, @@ -108,10 +160,10 @@ function mockDB (data, errors) { uid: data.uid }) }), - devices: sinon.spy(function () { + devices: sinon.spy(() => { return P.resolve(data.devices || []) }), - emailRecord: sinon.spy(function () { + emailRecord: sinon.spy(() => { if (errors.emailRecord) { return P.reject(errors.emailRecord) } @@ -122,31 +174,31 @@ function mockDB (data, errors) { email: data.email, emailVerified: data.emailVerified, kA: crypto.randomBytes(32), - lastAuthAt: function () { + lastAuthAt: () => { return Date.now() }, uid: data.uid, wrapWrapKb: crypto.randomBytes(32) }) }), - forgotPasswordVerified: sinon.spy(function () { + forgotPasswordVerified: sinon.spy(() => { return P.resolve(data.accountResetToken) }), - securityEvents: sinon.spy(function () { + securityEvents: sinon.spy(() => { return P.resolve([]) }), - sessions: sinon.spy(function () { + sessions: sinon.spy(() => { return P.resolve(data.sessions || []) }), - updateDevice: sinon.spy(function (uid, sessionTokenId, device) { + updateDevice: sinon.spy((uid, sessionTokenId, device) => { return P.resolve(device) }), - sessionTokenWithVerificationStatus: sinon.spy(function () { + sessionTokenWithVerificationStatus: sinon.spy(() => { return P.resolve({ tokenVerified: true }) }), - verifyTokens: sinon.spy(function () { + verifyTokens: sinon.spy(() => { if (errors.verifyTokens) { return P.reject(errors.verifyTokens) } @@ -156,12 +208,9 @@ function mockDB (data, errors) { } function mockObject (methodNames) { - return function (methods) { - return methodNames.reduce(function (object, name) { - object[name] = methods && methods[name] || sinon.spy(function () { - return P.resolve() - }) - + return methods => { + return methodNames.reduce((object, name) => { + object[name] = methods && methods[name] || sinon.spy(() => P.resolve()) return object }, {}) } @@ -171,14 +220,14 @@ function mockDevices (data) { data = data || {} return { - upsert: sinon.spy(function () { + upsert: sinon.spy(() => { return P.resolve({ id: data.deviceId || crypto.randomBytes(16), name: data.deviceName || 'mock device name', type: data.deviceType || 'desktop' }) }), - synthesizeName: sinon.spy(function () { + synthesizeName: sinon.spy(() => { return data.deviceName || null }) } @@ -188,8 +237,8 @@ function mockDevices (data) { // You can pass in an object of custom logging methods // if you need to e.g. make assertions about logged values. function mockLog (methods) { - var log = extend({}, methods) - LOG_METHOD_NAMES.forEach(function(name) { + const log = extend({}, methods) + LOG_METHOD_NAMES.forEach((name) => { if (!log[name]) { log[name] = function() {} } @@ -202,8 +251,9 @@ function mockLog (methods) { function spyLog (methods) { methods = extend({}, methods) methods.messages = methods.messages || [] - LOG_METHOD_NAMES.forEach(function(name) { + LOG_METHOD_NAMES.forEach(name => { if (!methods[name]) { + // arrow function would alter `this` inside the method methods[name] = function() { this.messages.push({ level: name,