Skip to content
This repository has been archived by the owner on Apr 3, 2019. It is now read-only.

Fixes issue #311 Use email buffer for DEL ‘/email/:email’ route #315

Merged
merged 2 commits into from
Mar 10, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions db-server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,17 +100,17 @@ function createServer(db) {

api.get('/email/:email',
op(function (req) {
return db.getSecondaryEmail(Buffer(req.params.email, 'hex'))
return db.getSecondaryEmail(req.params.email)
})
)
api.get('/email/:email/account',
op(function (req) {
return db.accountRecord(Buffer(req.params.email, 'hex'))
return db.accountRecord(req.params.email)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missed somehow, done now.

})
)
api.post('/email/:email/account/:id',
op(function (req) {
return db.setPrimaryEmail(req.params.id, Buffer(req.params.email, 'hex'))
return db.setPrimaryEmail(req.params.id, req.params.email)
})
)

Expand Down
18 changes: 11 additions & 7 deletions db-server/test/backend/db_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ const zeroBuffer16 = Buffer.from('00000000000000000000000000000000', 'hex')
const zeroBuffer32 = Buffer.from('0000000000000000000000000000000000000000000000000000000000000000', 'hex')
const now = Date.now()

function emailToHex(email) {
return Buffer.from(email).toString('hex')
}

function newUuid() {
return crypto.randomBytes(16)
}
Expand All @@ -35,7 +39,7 @@ function createAccount() {
locale: 'en_US',
}
account.normalizedEmail = account.email.toLowerCase()
account.emailBuffer = Buffer(account.email)
account.emailBuffer = Buffer.from(account.email)
return account
}

Expand Down Expand Up @@ -1399,7 +1403,7 @@ module.exports = function (config, DB) {
})

it('should get secondary email', () => {
return db.getSecondaryEmail(secondEmail.email)
return db.getSecondaryEmail(emailToHex(secondEmail.email))
.then((result) => {
assert.equal(result.email, secondEmail.email, 'matches secondEmail email')
assert.equal(!! result.isPrimary, false, 'isPrimary is false on secondEmail email')
Expand All @@ -1424,7 +1428,7 @@ module.exports = function (config, DB) {
})

it('should delete email', () => {
return db.deleteEmail(secondEmail.uid, secondEmail.email)
return db.deleteEmail(secondEmail.uid, emailToHex(secondEmail.email))
.then((result) => {
assert.deepEqual(result, {}, 'Returned an empty object on email deletion')

Expand Down Expand Up @@ -1482,7 +1486,7 @@ module.exports = function (config, DB) {
})

it('should fail to delete primary email', () => {
return db.deleteEmail(accountData.uid, accountData.normalizedEmail)
return db.deleteEmail(accountData.uid, emailToHex(accountData.normalizedEmail))
.then(assert.fail, (err) => {
assert.equal(err.errno, 136, 'should return email delete errno')
assert.equal(err.code, 400, 'should return email delete code')
Expand All @@ -1501,7 +1505,7 @@ module.exports = function (config, DB) {
})

it('should fail to get non-existent secondary email', () => {
return db.getSecondaryEmail('non-existent@email.com')
return db.getSecondaryEmail(emailToHex('non-existent@email.com'))
.then(assert.fail, (err) => {
assert.equal(err.errno, 116, 'should return not found errno')
assert.equal(err.code, 404, 'should return not found code')
Expand Down Expand Up @@ -1692,7 +1696,7 @@ module.exports = function (config, DB) {

it('should change a user\'s email', () => {
let sessionTokenData
return db.setPrimaryEmail(account.uid, secondEmail.email)
return db.setPrimaryEmail(account.uid, emailToHex(secondEmail.email))
.then(function (res) {
assert.deepEqual(res, {}, 'Returned an empty object on email change')
return db.accountEmails(account.uid)
Expand Down Expand Up @@ -1721,7 +1725,7 @@ module.exports = function (config, DB) {
assert.equal(session.email, secondEmail.email, 'should equal new primary email')
assert.deepEqual(session.emailCode, secondEmail.emailCode, 'should equal new primary emailCode')
assert.deepEqual(session.uid, account.uid, 'should equal account uid')
return P.all([db.accountRecord(secondEmail.email), db.accountRecord(account.email)])
return P.all([db.accountRecord(emailToHex(secondEmail.email)), db.accountRecord(emailToHex(account.email))])
})
.then((res) => {
assert.deepEqual(res[0], res[1], 'should return the same account record regardless of email used')
Expand Down
8 changes: 4 additions & 4 deletions db-server/test/backend/remote.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const P = require('../../../lib/promise')
const clientThen = require('../client-then')

function emailToHex(email) {
return Buffer(email).toString('hex')
return Buffer.from(email).toString('hex')
}

// Helper function that performs two tests:
Expand Down Expand Up @@ -192,7 +192,7 @@ module.exports = function(cfg, makeServer) {
assert.equal(!! result[2].isPrimary, false, 'isPrimary is false on thirdEmailRecord email')
assert.equal(!! result[2].isVerified, false, 'matches secondEmail thirdEmailRecord')

return client.delThen('/account/' + user.accountId + '/emails/' + secondEmailRecord.email)
return client.delThen('/account/' + user.accountId + '/emails/' + emailToHex(secondEmailRecord.email))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect! But we will also need to update the auth-server to pass an encoded email, similar to https://github.com/mozilla/fxa-auth-server/blob/45ae7b2048e9115590611a55454627d3840119a8/lib/db.js#L1069.

Ref: mozilla/fxa-auth-server#2334

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah sure i will complete it :)

})
.then(function (r) {
respOkEmpty(r)
Expand Down Expand Up @@ -228,7 +228,7 @@ module.exports = function(cfg, makeServer) {
return client.putThen('/account/' + user.accountId, user.account)
.then(function(r) {
respOkEmpty(r)
var randomPassword = Buffer(crypto.randomBytes(32)).toString('hex')
var randomPassword = Buffer.from(crypto.randomBytes(32)).toString('hex')
return client.postThen('/account/' + user.accountId + '/checkPassword', {'verifyHash': randomPassword})
.then(function(r) {
assert(false, 'should not be here, password isn\'t valid')
Expand Down Expand Up @@ -1180,7 +1180,7 @@ module.exports = function(cfg, makeServer) {
.then(
function (r) {
respOkEmpty(r)
return client.getThen('/emailBounces/' + Buffer(email).toString('hex'))
return client.getThen('/emailBounces/' + Buffer.from(email).toString('hex'))
}
)
.then(
Expand Down
2 changes: 1 addition & 1 deletion db-server/test/fake.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ function base64_16() { return base64(16) }
function base64_65() { return base64(65) }

function buf(len) {
return Buffer(crypto.randomBytes(len))
return Buffer.from(crypto.randomBytes(len))
}
function buf16() { return buf(16) }
function buf32() { return buf(32) }
Expand Down
8 changes: 6 additions & 2 deletions lib/db/mem.js
Original file line number Diff line number Diff line change
Expand Up @@ -1072,7 +1072,8 @@ module.exports = function (log, error) {
return P.resolve({})
}

Memory.prototype.getSecondaryEmail = function (emailBuffer) {
Memory.prototype.getSecondaryEmail = function (email) {
const emailBuffer = Buffer.from(email, 'hex')
const normalizedEmail = emailBuffer.toString('utf8').toLowerCase()

if (emails[normalizedEmail]) {
Expand All @@ -1082,7 +1083,8 @@ module.exports = function (log, error) {
}
}

Memory.prototype.accountRecord = function (emailBuffer) {
Memory.prototype.accountRecord = function (email) {
const emailBuffer = Buffer.from(email, 'hex')
const normalizedEmail = emailBuffer.toString('utf8').toLowerCase()

if (! emails[normalizedEmail]) {
Expand Down Expand Up @@ -1123,6 +1125,7 @@ module.exports = function (log, error) {
}

Memory.prototype.setPrimaryEmail = function (uid, email) {
email = Buffer.from(email, 'hex').toString('utf8')
if (! emails[email]) {
return P.reject(error.notFound())
}
Expand All @@ -1140,6 +1143,7 @@ module.exports = function (log, error) {
}

Memory.prototype.deleteEmail = function (uid, email) {
email = Buffer.from(email, 'hex').toString('utf8')
var emailRecord = emails[email]

if (emailRecord && emailRecord.uid.toString('hex') === uid.toString('hex') && emailRecord.isPrimary === false) {
Expand Down
13 changes: 8 additions & 5 deletions lib/db/mysql.js
Original file line number Diff line number Diff line change
Expand Up @@ -806,15 +806,17 @@ module.exports = function (log, error) {
// Get : email
// Values : email = $1
var GET_SECONDARY_EMAIL = 'CALL getSecondaryEmail_1(?)'
MySql.prototype.getSecondaryEmail = function (emailBuffer) {
MySql.prototype.getSecondaryEmail = function (email) {
const emailBuffer = Buffer.from(email, 'hex')
return this.readFirstResult(GET_SECONDARY_EMAIL, [emailBuffer.toString('utf8')])
}

// Select : accounts
// Fields : uid, email, normalizedEmail, emailVerified, emailCode, kA, wrapWrapKb, verifierVersion, authSalt, verifierSetAt, createdAt, lockedAt, primaryEmail
// Where : emails.normalizedEmail = LOWER($1)
var GET_ACCOUNT_RECORD = 'CALL accountRecord_2(?)'
MySql.prototype.accountRecord = function (emailBuffer) {
MySql.prototype.accountRecord = function (email) {
const emailBuffer = Buffer.from(email, 'hex')
return this.readFirstResult(GET_ACCOUNT_RECORD, [emailBuffer.toString('utf8')])
}

Expand All @@ -834,11 +836,12 @@ module.exports = function (log, error) {
// Values : uid = $1, email = $2
var SET_PRIMARY_EMAIL = 'CALL setPrimaryEmail_1(?, ?)'
MySql.prototype.setPrimaryEmail = function (uid, email) {
const emailBuffer = Buffer.from(email, 'hex')
return this.write(
SET_PRIMARY_EMAIL,
[
uid,
email
emailBuffer.toString('utf8')
]
)
}
Expand All @@ -847,11 +850,12 @@ module.exports = function (log, error) {
// Values : uid = $1, email = $2
var DELETE_EMAIL = 'CALL deleteEmail_2(?, ?)'
MySql.prototype.deleteEmail = function (uid, email) {
const emailBuffer = Buffer.from(email, 'hex')
return this.write(
DELETE_EMAIL,
[
uid,
email
emailBuffer.toString('utf8')
]
)
.catch(function(err){
Expand Down Expand Up @@ -1321,4 +1325,3 @@ module.exports = function (log, error) {

return MySql
}

5 changes: 2 additions & 3 deletions scripts/populate-session-tokens.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ var DB = require('../lib/db/mysql')(log, require('../db-server').errors)
var config = require('../config')
var crypto = require('crypto')

var zeroBuffer16 = Buffer('00000000000000000000000000000000', 'hex')
var zeroBuffer32 = Buffer('0000000000000000000000000000000000000000000000000000000000000000', 'hex')
var zeroBuffer16 = Buffer.from('00000000000000000000000000000000', 'hex')
var zeroBuffer32 = Buffer.from('0000000000000000000000000000000000000000000000000000000000000000', 'hex')

var count = parseInt(process.argv[2])

Expand Down Expand Up @@ -127,4 +127,3 @@ if (count > 0) {
} else {
throw new Error('Invalid argument')
}

6 changes: 3 additions & 3 deletions test/local/metrics_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ const crypto = require('crypto')
const proxyquire = require('proxyquire')
const sinon = require('sinon')

const zeroBuffer16 = Buffer('00000000000000000000000000000000', 'hex')
const zeroBuffer32 = Buffer('0000000000000000000000000000000000000000000000000000000000000000', 'hex')
const zeroBuffer16 = Buffer.from('00000000000000000000000000000000', 'hex')
const zeroBuffer32 = Buffer.from('0000000000000000000000000000000000000000000000000000000000000000', 'hex')

describe('DB metrics', () => {

Expand Down Expand Up @@ -344,7 +344,7 @@ describe('DB metrics', () => {
}

function hex (length) {
return Buffer(crypto.randomBytes(length).toString('hex'), 'hex')
return Buffer.from(crypto.randomBytes(length).toString('hex'), 'hex')
}

})