Skip to content
This repository has been archived by the owner. It is now read-only.
Permalink
Browse files
refactor(signin): Skip sign-in confirmation depending on account age (#…
…1591), r=@seanmonstar, @rfk

This PR adds the ability to skip sign-in confirmation depending on an account's age.
  • Loading branch information
vbudhram committed Dec 14, 2016
1 parent 68ae0ac commit 1d1fa419849e0950e9d09e03ccccef6f30849c4c
Showing with 117 additions and 0 deletions.
  1. +13 −0 config/index.js
  2. +16 −0 lib/routes/account.js
  3. +88 −0 test/local/account_routes.js
@@ -442,6 +442,19 @@ var conf = convict({
format: RegExp,
default: /.+@mozilla\.com$/,
env: 'SIGNIN_CONFIRMATION_FORCE_EMAIL_REGEX'
},
skipForNewAccounts: {
enabled :{

This comment has been minimized.

Copy link
@vladikoff

vladikoff Dec 20, 2016

Contributor

nit space: before : :o

This comment has been minimized.

Copy link
@rfk

rfk Dec 21, 2016

Member

🙀

doc: 'Skip sign-in confirmation for newly-created accounts.',
default: false,
env: 'SIGNIN_CONFIRMATION_SKIP_FOR_NEW_ACCOUNTS'
},
maxAge: {
doc: 'Maximum age at which an account is considered "new".',
format: 'duration',
default: '4 hours',
env: 'SIGNIN_CONFIRMATION_MAX_AGE_OF_NEW_ACCOUNTS'
}
}
},
securityHistory: {
@@ -659,6 +659,22 @@ module.exports = function (
return true
}
}

// If the account was recently created, don't make the user
// confirm sign-in for a configurable amount of time. This will reduce
// the friction of a user adding a second device.
const skipForNewAccounts = config.signinConfirmation.skipForNewAccounts
if (skipForNewAccounts && skipForNewAccounts.enabled) {
const accountAge = Date.now() - account.createdAt
if (accountAge <= skipForNewAccounts.maxAge) {
log.info({
op: 'account.signin.confirm.bypass.age',
uid: account.uid.toString('hex')
})
return true
}
}

return false
}

@@ -1171,6 +1171,94 @@ describe('/account/login', function () {
assert.equal(response.emailSent, true, 'response indicates an email was sent')
})
})

describe('skip for new accounts', function () {
function setup(enabled, accountCreatedSince) {
config.signinConfirmation.skipForNewAccounts = {
enabled: enabled,
maxAge: 5
}

mockDB.emailRecord = function () {
return P.resolve({
authSalt: crypto.randomBytes(32),
createdAt: Date.now() - accountCreatedSince,
data: crypto.randomBytes(32),
email: mockRequest.payload.email,
emailVerified: true,
kA: crypto.randomBytes(32),
lastAuthAt: function () {
return Date.now()
},
uid: uid,
wrapWrapKb: crypto.randomBytes(32)
})
}

var accountRoutes = makeRoutes({
checkPassword: function () {
return P.resolve(true)
},
config: config,
customs: mockCustoms,
db: mockDB,
log: mockLog,
mailer: mockMailer,
push: mockPush
})

route = getRoute(accountRoutes, '/account/login')
}

it('is disabled', function () {
setup(false)

return runTest(route, mockRequest, function (response) {
assert.equal(mockDB.createSessionToken.callCount, 1, 'db.createSessionToken was called')
var tokenData = mockDB.createSessionToken.getCall(0).args[0]
assert.ok(tokenData.mustVerify, 'sessionToken must be verified before use')
assert.ok(tokenData.tokenVerificationId, 'sessionToken was created unverified')
assert.equal(mockMailer.sendVerifyCode.callCount, 0, 'mailer.sendVerifyCode was not called')
assert.equal(mockMailer.sendNewDeviceLoginNotification.callCount, 0, 'mailer.sendNewDeviceLoginNotification was not called')
assert.ok(!response.verified, 'response indicates account is not verified')
assert.equal(response.verificationMethod, 'email', 'verificationMethod is email')
assert.equal(response.verificationReason, 'login', 'verificationReason is login')

assert.equal(mockMailer.sendVerifyLoginEmail.callCount, 1, 'mailer.sendVerifyLoginEmail was called')
assert.equal(mockMailer.sendVerifyLoginEmail.getCall(0).args[2].location.city, 'Mountain View')
assert.equal(mockMailer.sendVerifyLoginEmail.getCall(0).args[2].location.country, 'United States')
assert.equal(mockMailer.sendVerifyLoginEmail.getCall(0).args[2].timeZone, 'America/Los_Angeles')
})
})


it('skip sign-in confirmation on recently created account', function () {
setup(true, 0)

return runTest(route, mockRequest, function (response) {
assert.equal(mockDB.createSessionToken.callCount, 1, 'db.createSessionToken was called')
var tokenData = mockDB.createSessionToken.getCall(0).args[0]
assert.equal(tokenData.tokenVerificationId, null, 'sessionToken was created verified')
assert.equal(mockMailer.sendVerifyCode.callCount, 0, 'mailer.sendVerifyLoginEmail was not called')
assert.equal(mockMailer.sendNewDeviceLoginNotification.callCount, 1, 'mailer.sendNewDeviceLoginNotification was not called')
assert.ok(response.verified, 'response indicates account is verified')
})
})


it('don\'t skip sign-in confirmation on older account', function () {
setup(true, 10)

return runTest(route, mockRequest, function (response) {
assert.equal(mockDB.createSessionToken.callCount, 1, 'db.createSessionToken was called')
var tokenData = mockDB.createSessionToken.getCall(0).args[0]
assert.ok(tokenData.tokenVerificationId, 'sessionToken was created unverified')
assert.equal(mockMailer.sendVerifyLoginEmail.callCount, 1, 'mailer.sendVerifyLoginEmail was called')
assert.equal(mockMailer.sendNewDeviceLoginNotification.callCount, 0, 'mailer.sendNewDeviceLoginNotification was not called')
assert.ok(!response.verified, 'response indicates account is unverified')
})
})
})
})

it('creating too many sessions causes an error to be logged', function () {

0 comments on commit 1d1fa41

Please sign in to comment.